Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5762 closed defect (fixed)

SOCK_CLOEXEC should get ENOSYS, not EINVAL, on accept()

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In #5112 we added a bunch of checks to ignore open(), accept4(), etc failures if errno is EINVAL.

But it would appear the accept4() sets errno to ENOSYS, not EINVAL, if the system lacks O_CLOEXEC support.

A quick google led me to https://projects.honeynet.org/svn/sebek/virtualization/qebek/trunk/osdep.c which handles EINVAL in most cases but ENOSYS in the accept4() case.

Here's the simple patch, but it probably wants a changes stanza and a much better explanation in the comment if we like it:

diff --git a/src/common/compat.c b/src/common/compat.c
index fbb37ce..490deea 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -1016,7 +1016,7 @@ tor_accept_socket(tor_socket_t sockfd, struct sockaddr *
   /* If we got an error, see if it is EINVAL. EINVAL might indicate that,
    * event though we were built on a system with accept4 support, we
    * are running on one without. */
-  if (errno != EINVAL)
+  if (errno != ENOSYS)
     return s;
 #endif

See also http://osdir.com/ml/ruby-talk/2012-03/msg00082.html

Reported by murb.

Child Tickets

Attachments (1)

download.php?i=i5U8XqsU (498 bytes) - added by rransom 8 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 8 years ago by arma

Status: newneeds_review

comment:2 Changed 8 years ago by nickm

That's a tiny bit wrong. accept4 can give ENOSYS *or* EINVAL -- it gives ENOSYS if the accept4 syscall isn't supported by the kernel, and gives EINVAL if the syscall is present, but one the flags SOCK_CLOEXEC or SOCK_NONBLOCK isn't supported.

So instead of changing that line to "if (errno != ENOSYS)", we should technically change it to "if (errno != ENOSYS && errno != EINVAL)".

comment:3 Changed 8 years ago by arma

Great. So it would match the actual patch that murb tested: http://pastebin.com/i5U8XqsU

You actually know what you're doing here, so I'll let you take it from here. :)

comment:4 Changed 8 years ago by arma

Status: needs_reviewneeds_revision

Changed 8 years ago by rransom

Attachment: download.php?i=i5U8XqsU added

comment:5 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_revisionclosed

Merged; thanks!

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.