Bug 1470310

Summary: gedit default save as location is / in 7.4
Product: Red Hat Enterprise Linux 7 Reporter: Paul Gozart <pgozart>
Component: dbusAssignee: David King <dking>
Status: CLOSED ERRATA QA Contact: Desktop QE <desktop-qa-list>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.4CC: briang, dking, jhunt, pschindl, rstrode, smoroney, tpelka, vanhoof
Target Milestone: rcKeywords: Regression
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: dbus-1.10.24-12.el7 Doc Type: No Doc Update
Doc Text:
undefined
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-10-30 11:42:30 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 1546815, 1571842    
Attachments:
Description Flags
dist-git patch with the HOME chdir change
none
fix dbus-daemon --fork too none

Description Paul Gozart 2017-07-12 17:50:51 UTC
Description of problem:
When using "Save as" to save a file with Gedit the default loction is the / directory instead of the users home directory.


Version-Release number of selected component (if applicable):
Latest 7.4 Beta release


How reproducible:
Always


Steps to Reproduce:
1.  Install latest RHEL 7.4 Beta Workstation
2.  As normal user, open gedit and choose 'Save As' from menu


Actual results:
The default save location is /


Expected results:
The default save location is /home/$user

Comment 2 Paul Gozart 2017-07-13 15:26:04 UTC
customer is seeing this in RHEL 7.4 beta.  the package is gedit-3.22.0-3.el7.x86_64

Comment 3 Paul Gozart 2017-10-12 22:13:46 UTC
** This is also a problem in RHEL 7.4 GA. **

Comment 6 Ray Strode [halfline] 2018-06-20 19:14:06 UTC
I did a little digging into this today.

The issue is, that in 7.4 gedit started being dbus activated instead of getting run directly.  The dbus-daemon has a current working directory of /, so gedit inherits that.  The file chooser saves to the current working directory by default.

The reason that dbus-daemon has a current working directory of /, is because of this code in dbus-launch.c:

  /* We chdir ("/") since we are persistent and daemon-like, and fork•
   * again so dbus-launch can reap the parent.  However, we don't•
   * setsid() or close fd 0 because the idea is to remain attached•
   * to the tty and the X server in order to kill the message bus•
   * when the session ends.•
   */•
•
  if (chdir ("/") < 0)•

So it's doing the chdir("/") because that's something daemon's often do, and it's trying to be daemon-like, and that chunk of code has existed since the initial implementation.

But, that rationale is shaky at best.  daemon's chdir to the filesystem root so they don't end up occupying a mount point and unintentionally make that mountpoint unmountable. dbus-launch is used for launching _session buses_ which should be scoped to the user session and by extension the user's home directory.

Possible fixes:

1) drop the chdir from dbus-launch.c
2) change the xinit scripts to use dbus-run-session instead of dbus-launch
3) make gedit no longer dbus activatable
4) hard code the save folder in gedit to $HOME

I don't like 3 or 4 because, other applications are probably effected as well.  a number of gnome applications got rebased in 7.4 and are now dbus activatable.

Reassigning to David King to get his take on if 1 or 2 is better.

Comment 7 David King 2018-06-21 10:03:20 UTC
(In reply to Ray Strode [halfline] from comment #6)
> I did a little digging into this today.
> 
> The issue is, that in 7.4 gedit started being dbus activated instead of
> getting run directly.  The dbus-daemon has a current working directory of /,
> so gedit inherits that.  The file chooser saves to the current working
> directory by default.
> 
> The reason that dbus-daemon has a current working directory of /, is because
> of this code in dbus-launch.c:
> 
>   /* We chdir ("/") since we are persistent and daemon-like, and fork•
>    * again so dbus-launch can reap the parent.  However, we don't•
>    * setsid() or close fd 0 because the idea is to remain attached•
>    * to the tty and the X server in order to kill the message bus•
>    * when the session ends.•
>    */•
> •
>   if (chdir ("/") < 0)•
> 
> So it's doing the chdir("/") because that's something daemon's often do, and
> it's trying to be daemon-like, and that chunk of code has existed since the
> initial implementation.
> 
> But, that rationale is shaky at best.  daemon's chdir to the filesystem root
> so they don't end up occupying a mount point and unintentionally make that
> mountpoint unmountable. dbus-launch is used for launching _session buses_
> which should be scoped to the user session and by extension the user's home
> directory.
> 
> Possible fixes:
> 
> 1) drop the chdir from dbus-launch.c
> 2) change the xinit scripts to use dbus-run-session instead of dbus-launch
> 3) make gedit no longer dbus activatable
> 4) hard code the save folder in gedit to $HOME
> 
> I don't like 3 or 4 because, other applications are probably effected as
> well.  a number of gnome applications got rebased in 7.4 and are now dbus
> activatable.
> 
> Reassigning to David King to get his take on if 1 or 2 is better.

dbus-run-session is not a drop-in replacement for dbus-launch, and in this case is probably not suitable. It is intended for running another binary under a new dbus session bus, regardless of whether a session bus already exists. It does not accept no arguments, as with dbus-launch, for launching a session bus conveniently and outputting the bus addresses. dbus-run-session is better summarised as a tool for running and automatically terminating a session bus in SSH sessions, or for having an isolated session bus in regression test scenarios. The SSH sessions use case is what caused bug 1268972 to be filed, to get dbus-run-session into the dbus in RHEL 6.

In the case of RHEL 7, as we do not have systemd as a user session manager, it is gnome-session that calls dbus-launch to start the session bus. Additionally, there is an xinit script (/etc/X11/xinit/xinitrc.d/00-start-message-bus.sh) which does a similar thing. In other words, once RHEL uses systemd as a user session manager, this bug will disappear, because dbus-daemon for the session bus will be launched by systemd, with the user's home directory as the current working directory.

In summary, for RHEL 7, I think that dropping the chdir() call in dbus-launch is the best fix. I agree completely with your reasoning about dbus-launch only being used to launch session buses, and that acting like other session-scoped processes is the path of least surprise. I think that it also makes sense for an upstream bug against dbus to remove the chdir() call, so I will file one.

Comment 8 Ray Strode [halfline] 2018-06-21 13:18:50 UTC
it would be possible to use dbus-run-session by dropping 00-start-msesage-bus.sh and changing /etc/X11/xinit/Xsession to use dbus-run-session in the same way CK_XINIT_SESSION and SSH_AGENT are used (where the variables are empty if they aren't supposed to get called).

anyway, changing dbus-launch seems fine to me. Simon's point on the upstream bug is reasonable, though. dbus-launch can get called from libdbus too.  so maybe instead of dropping the chdir it should change it to $HOME.  Or still run it in the --autolaunch case.

Comment 9 David King 2018-06-22 14:39:04 UTC
(In reply to Ray Strode [halfline] from comment #8)
> anyway, changing dbus-launch seems fine to me. Simon's point on the upstream
> bug is reasonable, though. dbus-launch can get called from libdbus too.  so
> maybe instead of dropping the chdir it should change it to $HOME.  Or still
> run it in the --autolaunch case.

I updated the patch to use HOME if its set. I am unsure if we want to deal with the autolaunch case specifically, so best to wait for Simon's opinion on the patch, I think.

Comment 10 David King 2018-06-22 15:59:54 UTC
Created attachment 1453778 [details]
dist-git patch with the HOME chdir change

I'm about to run out of battery, but this patch seems to work. I built a scratch build with it successfully, but I won't get mains back for a few hours, so feel free to push and build it if you think it makes sense before then.

Comment 13 Petr Schindler 2018-08-21 13:54:08 UTC
This bug still appears with versions:
dbus-1.10.24-11.el7.x86_64
gedit-3.28.1-1.el7.x86_64

When I save newly created document, the location still defaults to /. I tried to remove ~/.config/gedit and it didn't help.

Comment 14 Ray Strode [halfline] 2018-08-21 14:53:13 UTC
so indeed, looking at a 7.6 VM we see the patch doesn't work:

$ ps -ef |grep daemon-daemon
rstrode   3900     1  0 10:45 ?        00:00:00 /usr/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session

$ ls -l /proc/3900/cwd
lrwxrwxrwx. 1 rstrode rstrode 0 Aug 21 10:50 /proc/3900/cwd -> /

dbus-launch is right now, though, so it's not a case of the patch not getting applied:

$ ps -ef |grep dbus-launch

rstrode   2800     1  0 10:38 ?        00:00:00 dbus-launch --sh-syntax --exit-with-session

$ ls -l /proc/2800/cwd
lrwxrwxrwx. 1 rstrode rstrode 0 Aug 21 10:43 /proc/2800/cwd -> /home/rstrode

In that first ps output we see dbus-daemon is called with --fork.  Looking in the code, dbus-daemon calls `_dbus_become_daemon` when invoked with --fork. that function does this:

```
  _dbus_verbose ("chdir to /\n");•
  if (chdir ("/") < 0)•
    {•
      dbus_set_error (error, DBUS_ERROR_FAILED,•
                      "Could not chdir() to root directory");•
      return FALSE;•
    }•
```

Comment 15 Ray Strode [halfline] 2018-08-21 15:29:42 UTC
Created attachment 1477617 [details]
fix dbus-daemon --fork too

I've confirmed in testing that fixing the above code, addresses the problem with gedit.

Building in the build system now and moving back to MODIFIED.

Comment 17 Petr Schindler 2018-08-22 05:53:14 UTC
dbus-1.10.24-12.el7 fixes the bug. Default location is now user's home.

Comment 23 errata-xmlrpc 2018-10-30 11:42:30 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:3288