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 544672 - "seahorse-agent --execute" leaks file descriptors
"seahorse-agent --execute" leaks file descriptors
Status: RESOLVED FIXED
Product: seahorse-plugins
Classification: Applications
Component: Agent
2.22.x
Other Linux
: Normal major
: 2.24
Assigned To: seahorse-plugins-maint
seahorse-plugins-maint
Depends on:
Blocks:
 
 
Reported: 2008-07-25 11:05 UTC by Josselin Mouette
Modified: 2008-07-27 16:49 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
sets appropriate flag (1.44 KB, patch)
2008-07-27 03:06 UTC, Adam Schreiber
committed Details | Review
Set FD_CLOEXEC on the agent socket (1.42 KB, patch)
2008-07-27 05:05 UTC, Josselin Mouette
committed Details | Review

Description Josselin Mouette 2008-07-25 11:05:56 UTC
[ Forwarded from http://bugs.debian.org/492282 by Stefan Fritsch ]

Seahorse leaks file descriptors to processes started with "seahorse-agent
--execute", including the gpg agent listening socket. For the default setup,
this means that all processes started from the desktop inherit those FDs and can
possibly use them. This can be a security issue because the FDs are also
inherited to processes started with su as a different user which normally would
not have access to gpg key and gpg agent socket.

Seahorse should use fcntl to set FD_CLOEXEC on its FDs.

[ Additionnal comments ]

In Debian, the session is initiated by “seahorse-agent --execute gnome-session”. This means the file descriptors leak to gnome-session (this can be easily confirmed). However it seems that gnome-session correctly closes these file descriptors before spawning anything else, so this is not as problematic as the original submitter says.
Comment 1 Adam Schreiber 2008-07-27 03:06:41 UTC
Created attachment 115357 [details] [review]
sets appropriate flag

Please test this patch.  seahorse-agent still works properly on my system with it.
Comment 2 Josselin Mouette 2008-07-27 04:24:51 UTC
I don’t think this will fix the issue, since this only adds the FD_CLOEXEC flag on stdin/stdout/stderr, which are /dev/null anyway. The problem comes from other file descriptors opened before the fork.
Comment 3 Adam Schreiber 2008-07-27 04:42:01 UTC
Those are the only calls to open () that I found in the code except for a call in libseahorse/seahorse-gpg-options.c that is closed immediately.  Unless gio opens some file descriptors that aren't exposed to the user, I think I've covered our descriptors.  If you could point out any I've missed, I would appreciate it.
Comment 4 Josselin Mouette 2008-07-27 05:05:35 UTC
Created attachment 115359 [details] [review]
Set FD_CLOEXEC on the agent socket

(In reply to comment #3)
> Those are the only calls to open () that I found in the code except for a call
> in libseahorse/seahorse-gpg-options.c that is closed immediately.

There are not only open(), but also socket() and pipe().

This patch fixes the leak of the agent socket. There are also three pipes opened that are leaked. I guess one of them is the signal pipe, but I can’t find the others.
Comment 5 Josselin Mouette 2008-07-27 05:59:37 UTC
I can tell the signal pipe is innocent, so this means these pipes are leaked by gpgme; it seems they are created when it calls gnupg, and not closed after. 
Comment 6 Stef Walter 2008-07-27 16:07:47 UTC
Patches look good to commit. Not sure about using g_critical if it fails. We may want to make that a g_warning, since seahorse builds on all sorts of strange OS's. 
Comment 7 Adam Schreiber 2008-07-27 16:49:33 UTC
Committed.  I changed J's to use warning instead of critical and added the return value checking to mine.

2008-07-27  Adam Schreiber  <sadam@clemson.edu>
    
    * agent/seahorse-agent-main.c:
    * agent/seahorse-agent-io.c: Close file descriptors after fork
    Patch from Josselin Mouette.  Fixes bug #544672