Bug 1470310 - gedit default save as location is / in 7.4
Summary: gedit default save as location is / in 7.4
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: dbus
Version: 7.4
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: ---
Assignee: David King
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks: 1546815 1571842
TreeView+ depends on / blocked
 
Reported: 2017-07-12 17:50 UTC by Paul Gozart
Modified: 2018-10-30 11:42 UTC (History)
8 users (show)

Fixed In Version: dbus-1.10.24-12.el7
Doc Type: No Doc Update
Doc Text:
undefined
Clone Of:
Environment:
Last Closed: 2018-10-30 11:42:30 UTC


Attachments (Terms of Use)
dist-git patch with the HOME chdir change (5.18 KB, patch)
2018-06-22 15:59 UTC, David King
no flags Details | Diff
fix dbus-daemon --fork too (16.92 KB, patch)
2018-08-21 15:29 UTC, Ray Strode [halfline]
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2018:3288 None None None 2018-10-30 11:42:57 UTC
FreeDesktop.org 106987 None None None 2018-06-21 10:27:57 UTC
Red Hat Knowledge Base (Solution) 3147481 None None None 2017-10-12 21:55:36 UTC

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


Note You need to log in before you can comment on or make changes to this bug.