| Summary: | [PATCH] fix/improve SOCK_CLOEXEC handling when defined but not supported by socket/socketpair | ||
|---|---|---|---|
| Product: | dbus | Reporter: | Pino Toscano <toscano.pino> |
| Component: | core | Assignee: | Havoc Pennington <hp> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | chengwei.yang.cn |
| Version: | 1.4.x | Keywords: | patch |
| Hardware: | All | ||
| OS: | other | ||
| Whiteboard: | review+ | ||
| i915 platform: | i915 features: | ||
| Attachments: |
Check also for EPROTOTYPE
[PATCH 1/2] Check EINVAL for socketpair and retry without SOCK_CLOEXEC [PATCH 2/2] Allow EPROTOTYPE for SOCK_CLOEXEC but unsupported by socket/socketpair |
||
Comment on attachment 85398 [details]
Check also for EPROTOTYPE
Pino Toscano, I'll upload a V2 of your patch with the same check for _dbus_connect_exec() based on a fix for _dbus_connect_exec().
Created attachment 85564 [details] [review] [PATCH 1/2] Check EINVAL for socketpair and retry without SOCK_CLOEXEC Created attachment 85565 [details] [review] [PATCH 2/2] Allow EPROTOTYPE for SOCK_CLOEXEC but unsupported by socket/socketpair (In reply to comment #1) > Pino Toscano, I'll upload a V2 of your patch with the same check for > _dbus_connect_exec() based on a fix for _dbus_connect_exec(). Your patch and the updated version of mine look okay, thanks! Comment on attachment 85564 [details] [review] [PATCH 1/2] Check EINVAL for socketpair and retry without SOCK_CLOEXEC Review of attachment 85564 [details] [review]: ----------------------------------------------------------------- ::: dbus/dbus-sysdeps-unix.c @@ +902,2 @@ > #ifdef SOCK_CLOEXEC > + retval = socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, fds); socketpair ( (with the space) @@ +902,3 @@ > #ifdef SOCK_CLOEXEC > + retval = socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, fds); > + cloexec_done = retval >= 0; I'd prefer cloexec_done = (retval >= 0) to make the precedence obvious. @@ +907,3 @@ > #endif > + { > + retval = socketpair(AF_UNIX, SOCK_STREAM, 0, fds); with a space again Applied to my local branch with trivial coding style fixes, will merge after testing Fixed in git for 1.6.16, 1.7.6. Thanks! |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 85398 [details] Check also for EPROTOTYPE _dbus_open_socket and _dbus_full_duplex_pipe in dbus-sysdeps-unix.c can make use of SOCK_CLOEXEC if defined, trying to use it and eventually setting the cloexec bit if not succeeded at creation time. Although, if SOCK_CLOEXEC is defined (most probably because accept4 is implemented) but not supported by socket and socketpair yet, only EINVAL (the return value on Linux) is checked, while the canonical POSIX errno value for an invalid socket type is EPROTOTYPE. The above is what happens on GNU/Hurd after accept4 (and thus SOCK_* flags) was added in glibc. Attached there is a small commit (based on the dbus-1.6 branch) which checks for EPROTOTYPE in addition to EINVAL. (Most probably also _dbus_connect_exec should have a similar runtime check wrt the socketpair+SOCK_CLOEXEC failure as _dbus_open_socket and _dbus_full_duplex_pipe do, but that could be done later if needed, I guess.)