After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793445 - Refusing to render service to dead parents.
Refusing to render service to dead parents.
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: admin backend
git master
Other Linux
: Normal normal
: ---
Assigned To: Cosimo Cecchi
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2018-02-14 12:16 UTC by Ondrej Holy
Modified: 2018-02-16 15:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
admin: Use really persistent d-bus name (1.54 KB, patch)
2018-02-14 12:24 UTC, Ondrej Holy
committed Details | Review
admin: Add pkexec wrapper script (2.98 KB, patch)
2018-02-14 12:24 UTC, Ondrej Holy
none Details | Review
daemon: Fix admin backend spawning (2.03 KB, patch)
2018-02-15 09:52 UTC, Ondrej Holy
none Details | Review
daemon: Fix admin backend spawning (2.24 KB, patch)
2018-02-16 15:40 UTC, Ondrej Holy
committed Details | Review

Description Ondrej Holy 2018-02-14 12:16:03 UTC
Admin backend doesn't work on some systems due to "Refusing to render service to dead parents." pkexec error. Pkexec can't figure out the parent process:
https://cgit.freedesktop.org/polkit/tree/src/programs/pkexec.c#n727

I'm just wondering why that approach works e.g. on Fedora and not e.g. in RHEL/CentOS.

See:
https://mail.gnome.org/archives/gvfs-list/2018-February/msg00000.html
Comment 1 Ondrej Holy 2018-02-14 12:24:36 UTC
Created attachment 368340 [details] [review]
admin: Use really persistent d-bus name

Commit 8e9439ef introduced DBusName=org.gtk.vfs.mountpoint_admin
in admin.mount.in, but forgot to set the necessary mount options.
So, each client spawns new daemon currently, which is not necessary.
Let's set the missing -DMOUNTABLE_DBUS_NAME options.
Comment 2 Ondrej Holy 2018-02-14 12:24:42 UTC
Created attachment 368341 [details] [review]
admin: Add pkexec wrapper script

pkexec fails on some systems (e.g. RHEL) with "Refusing to render
service to dead parents." error currently. Let's introduce
gvfsd-admin-pkexec wrapper script which workarounds this issue.
Comment 3 Ondrej Holy 2018-02-14 12:26:00 UTC
However, I would be happier if the wrapper doesn't have to be added. Any ideas?
Comment 4 Ondrej Holy 2018-02-14 12:28:00 UTC
Any idea what is a difference between "sh -c" and "#!/bin/sh" script in regards to ppid?
Comment 5 Ondrej Holy 2018-02-14 14:02:55 UTC
It seems that G_SPAWN_DO_NOT_REAP_CHILD when spawning may fix the issue without adding the wrapper. I will test later...
Comment 6 Ondrej Holy 2018-02-15 09:52:38 UTC
Created attachment 368367 [details] [review]
daemon: Fix admin backend spawning

pkexec fails on some systems with "Refusing to render service to dead
parents.", which is caused by double forking when spawning the process.
Let's prevent this by G_SPAWN_DO_NOT_REAP_CHILD flag and clean up
manually using g_child_watch_add.
Comment 7 Ondrej Holy 2018-02-15 11:28:23 UTC
The second approach seems to fix this issue also and do not require additional files, though not sure that it doesn't have any unwanted side-effects... Philip, don't you know?
Comment 8 Ondrej Holy 2018-02-15 11:29:16 UTC
Comment on attachment 368340 [details] [review]
admin: Use really persistent d-bus name

Attachment 368340 [details] pushed as 2502641 - admin: Use really persistent d-bus name
Comment 9 Kenneth Swarthout 2018-02-15 12:51:07 UTC
(In reply to Ondrej Holy from comment #3)
> However, I would be happier if the wrapper doesn't have to be added. Any
> ideas?

I would like, very much, not to have the wrapper if there's any ways around it.
Comment 10 Kenneth Swarthout 2018-02-15 13:01:27 UTC
(In reply to Ondrej Holy from comment #4)
> Any idea what is a difference between "sh -c" and "#!/bin/sh" script in
> regards to ppid?

According to https://www.gnu.org/software/bash/manual/html_node/Bash-Variables.html#index-PPID :

PPID
The process ID of the shell’s parent process. This variable is readonly.

So I don't think there would be any difference in regards to ppid, except for maybe sh, on some systems, might run the real sh, whereas on my system, #!/bin/sh is a symbolic-link to a /bin/bash.    which sh  shows sh is /usr/bin/sh, which happens to also be a symbolic link to /bin/bash.

Are you seeing a difference in behaviour when you use sh -c versus #!/bin/sh with regards to the PPID variable?
Comment 11 Philip Withnall 2018-02-15 13:41:46 UTC
Review of attachment 368367 [details] [review]:

(In reply to Ondrej Holy from comment #7)
> The second approach seems to fix this issue also and do not require
> additional files, though not sure that it doesn't have any unwanted
> side-effects... Philip, don't you know?

I don’t really know what the situation is here. If this works, go for it. I can’t think of any side effects which it could cause (honestly, I thought that the code you’ve added with g_child_watch_add() is equivalent to not passing the G_SPAWN_DO_NOT_REAP_CHILD_FLAG). Looking at gspawn.c, it seems that if you don’t pass G_SPAWN_DO_NOT_REAP_CHILD, an intermediate child process is spawned, and the process you actually want to spawn becomes your grandchild. That might have been interfering with the ppid.

::: daemon/mount.c
@@ +475,1 @@
       /* TODO: Add a timeout here to detect spawned app crashing */

Implementing this TODO will be possible now that you’re using g_child_watch_add() (since the watch callback will be called if the child crashes).
Comment 12 Ondrej Holy 2018-02-16 15:40:11 UTC
Philip, thanks, I have understood it, in the same way, though still wonder why it currently works on Fedora and not in RHEL and possibly in others :-)
Comment 13 Ondrej Holy 2018-02-16 15:40:54 UTC
Created attachment 368428 [details] [review]
daemon: Fix admin backend spawning

pkexec fails on some systems with "Refusing to render service to dead
parents.", which is caused by double forking when spawning the process.
Let's prevent this by G_SPAWN_DO_NOT_REAP_CHILD flag and clean up
manually using g_child_watch_add.
Comment 14 Ondrej Holy 2018-02-16 15:41:42 UTC
Attachment 368428 [details] pushed as cb1c755 - daemon: Fix admin backend spawning