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 649480 - Use MSG_CMSG_CLOEXEC in recvmsg in gio/gsocket.c
Use MSG_CMSG_CLOEXEC in recvmsg in gio/gsocket.c
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-05-05 16:58 UTC by Kristian Høgsberg
Modified: 2011-05-05 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GSocket: Use MSG_CMSG_CLOEXEC (1.34 KB, patch)
2011-05-05 17:19 UTC, Colin Walters
none Details | Review
GSocket: Use MSG_CMSG_CLOEXEC (2.89 KB, patch)
2011-05-05 17:38 UTC, Colin Walters
reviewed Details | Review
GSocket: Use MSG_CMSG_CLOEXEC (1.96 KB, patch)
2011-05-05 18:09 UTC, Colin Walters
committed Details | Review

Description Kristian Høgsberg 2011-05-05 16:58:57 UTC
The gsocket implementation should use the MSG_CMSG_CLOEXEC flag to recvmsg to receive the passed fds with the CLOEXEC flag already set when possible.

This flag is useful for the  same  reasons as the O_CLOEXEC flag of open(2)

See recvmsg(2).
Comment 1 Colin Walters 2011-05-05 17:19:25 UTC
Created attachment 187300 [details] [review]
GSocket: Use MSG_CMSG_CLOEXEC

This ensures received file descriptors don't leak to child processes.
Comment 2 Colin Walters 2011-05-05 17:38:56 UTC
Created attachment 187306 [details] [review]
GSocket: Use MSG_CMSG_CLOEXEC

Updates from IRC review by krh
Comment 3 Colin Walters 2011-05-05 17:50:19 UTC
For posterity:

<walters> krh: bam, a patch
<krh> walters: wowzers
<walters> it's danw's ball probably
<krh> woo, splinter!
* krh reviews
<krh> walters: well, basically, my comment is to just skip the fcntl loop if we have the CLOEXEC flag
<walters> krh: hm though there is one detail here; we get complaints from people compiling against newer headers but running on old kernels
 this happens in autobuilder type scenarios
 we may need to check for EINVAL
 and if we have to handle that, we might as well do the fcntl
<krh> walters: doesn't glibc handle that?
<krh> fall back and emulate with fcntl internally
<krh> walters: the other comments was, don't clear MSG_CMSG_CLOEXEC if the user set it in *flags
<walters> from my glibc checkout as of sep 8 2010, nope
 oh true
Comment 4 Dan Winship 2011-05-05 17:57:48 UTC
Comment on attachment 187306 [details] [review]
GSocket: Use MSG_CMSG_CLOEXEC


>+#ifdef MSG_CMSG_CLOEXEC	
>+	if (result < 0 && get_socket_errno () == EINVAL)

Arguably "&& !user_specified_cmsg_cloexec", but I guess we can just say that gio abstracts away the old kernel problem for the caller.

>+	    /* Filter out the Linux-only flag we added above; unless

you don't need this part; recvmsg() overwrites msg.msg_flags
Comment 5 Colin Walters 2011-05-05 18:09:46 UTC
Created attachment 187308 [details] [review]
GSocket: Use MSG_CMSG_CLOEXEC

Updated for danw's comments
Comment 6 Colin Walters 2011-05-05 18:31:07 UTC
Attachment 187308 [details] pushed as 8932a1a - GSocket: Use MSG_CMSG_CLOEXEC