Bug 1470310
| Summary: | gedit default save as location is / in 7.4 | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Paul Gozart <pgozart> | ||||||
| Component: | dbus | Assignee: | David King <dking> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Desktop QE <desktop-qa-list> | ||||||
| Severity: | medium | Docs Contact: | |||||||
| Priority: | medium | ||||||||
| Version: | 7.4 | CC: | bgollahe, dking, jhunt, pschindl, rstrode, smoroney, tpelka, vanhoof | ||||||
| Target Milestone: | rc | Keywords: | 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: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | |||||||||
| Bug Blocks: | 1546815, 1571842 | ||||||||
| Attachments: |
|
||||||||
|
Description
Paul Gozart
2017-07-12 17:50:51 UTC
customer is seeing this in RHEL 7.4 beta. the package is gedit-3.22.0-3.el7.x86_64 ** This is also a problem in RHEL 7.4 GA. ** 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.
(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. 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. (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. 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.
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. 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;•
}•
```
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.
dbus-1.10.24-12.el7 fixes the bug. Default location is now user's home. 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 |