This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] handle EPROTOTYPE for socket invocations with SOCK_* flags


Alle giovedì 7 febbraio 2013, Roland McGrath ha scritto:
> > The snippets of fallback code that handle failures of invocation of
> > socket with SOCK_CLOEXEC or SOCK_NONBLOCK seem to not correctly
> > disable have_sock_cloexec/__have_sock_cloexec if errno is
> > EPROTOTYPE, as returned when the socket type (like "SOCK_STREAM |
> > SOCK_CLOEXEC" for socket with no handling of flags) is unknown.
> 
> There seem to be multiple errno values that could indicate this. 
> POSIX specifies that socket shall fail with EPROTOTYPE if "The
> socket type is not supported by the protocol."  This could be
> construed as applying only if the type value is valid in general and
> is not supported by the particular protocol.  By that reading, it
> still conforms to POSIX if socket uses any errno code it likes
> (except those specified for other error cases of socket, like
> EAFNOSUPPORT; EINVAL is not specified for socket).

I'm not sure to agree with this interpretation though, since IMHO the 
validation of the socket type should depend on the previous validation 
of the protocol.

> Arguably we should harmonize the Hurd's use of errno codes here
> either to Linux or to BSD.  That is, unless someone gets a
> clarification from the POSIX committee and that says that the
> behavior is nonconforming.  (I'm not really clear on how it could
> be, since you have to write a nonconforming program to elicit these
> errors.)

Please bear with me, but I still don't understand what would be the 
issue with Hurd's errno codes, in this case; are you implying Hurd 
should behave e.g. like Linux and return EINVAL for "high" invalid 
values of socket types?

Updated version of the patch: I introduced a new __-prefixed function 
versioned as GLIBC_PRIVATE to be used also outside libc (libresolv and 
nscd), I hope to have done it correctly.

-- 
Pino Toscano
Handle more errors for socket invocations with SOCK_* flags

If SOCK_CLOEXEC and SOCK_NONBLOCK are defined but not __ASSUME_SOCK_CLOEXEC,
trying to use them as socket type in invocations of socket will return
EPROTOTYPE, which is not handled and thus making them behave as if those flags
were actually supported.

Add a new __is_socket_error function to check for recognized socket errors,
i.e. ESOCKTNOSUPPORT, EPROTOTYPE, and EPROTONOSUPPORT in addition to EINVAL.

2013-02-07  Pino Toscano  <toscano.pino@tiscali.it>

	* include/sys/socket.h: Include <stdbool.h>
	(__is_socket_error): New prototype.
	* socket/have_sock_cloexec.c: Include <errno.h>
	[SOCK_CLOEXEC && !__ASSUME_SOCK_CLOEXEC] (__is_socket_error):
	New function.
	* socket/Versions (GLIBC_PRIVATE): Add __is_socket_error.
	* nscd/connections.c (nscd_init) [!__ASSUME_SOCK_CLOEXEC]:
	Use __is_socket_error.
	* nscd/nscd_helper.c (open_socket) [SOCK_CLOEXEC]
	[!__ASSUME_SOCK_CLOEXEC]: Likewise.
	* resolv/res_send.c (reopen) [!__ASSUME_SOCK_CLOEXEC]: Likewise.
	* sunrpc/clnt_udp.c (__libc_clntudp_bufcreate) [SOCK_NONBLOCK]
	[!__ASSUME_SOCK_CLOEXEC]: Likewise.
	* misc/syslog.c (openlog_internal) [SOCK_CLOEXEC]
	[!__ASSUME_SOCK_CLOEXEC]: Likewise.

--- a/include/sys/socket.h
+++ b/include/sys/socket.h
@@ -1,5 +1,6 @@
 #ifndef _SYS_SOCKET_H
 #include <socket/sys/socket.h>
+#include <stdbool.h>
 
 #ifndef _ISOMAC
 /* Now define the internal interfaces.  */
@@ -153,6 +154,7 @@ libc_hidden_proto (__libc_sa_len)
 
 #ifdef SOCK_CLOEXEC
 extern int __have_sock_cloexec attribute_hidden;
+extern bool __is_socket_error (int err);
 /* At lot of other functionality became available at the same time as
    SOCK_CLOEXEC.  Avoid defining separate variables for all of them
    unless it is really necessary.  */
--- a/misc/syslog.c
+++ b/misc/syslog.c
@@ -357,7 +357,7 @@ openlog_internal(const char *ident, int
 					if (__have_sock_cloexec == 0)
 						__have_sock_cloexec
 						  = ((LogFile != -1
-						      || errno != EINVAL)
+						      || !__is_socket_error (errno))
 						     ? 1 : -1);
 				}
 # endif
--- a/nscd/connections.c
+++ b/nscd/connections.c
@@ -856,7 +856,7 @@ cannot set socket to close on exec: %s;
       sock = socket (AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
 #ifndef __ASSUME_SOCK_CLOEXEC
       if (have_sock_cloexec == 0)
-	have_sock_cloexec = sock != -1 || errno != EINVAL ? 1 : -1;
+	have_sock_cloexec = sock != -1 || !__is_socket_error (errno) ? 1 : -1;
 #endif
     }
 #ifndef __ASSUME_SOCK_CLOEXEC
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -172,7 +172,7 @@ open_socket (request_type type, const ch
       sock = __socket (PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
 # ifndef __ASSUME_SOCK_CLOEXEC
       if (__have_sock_cloexec == 0)
-	__have_sock_cloexec = sock != -1 || errno != EINVAL ? 1 : -1;
+	__have_sock_cloexec = sock != -1 || !__is_socket_error (errno) ? 1 : -1;
 # endif
     }
 #endif
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -925,7 +925,7 @@ reopen (res_state statp, int *terrno, in
 				if (__have_o_nonblock == 0)
 					__have_o_nonblock
 					  = (EXT(statp).nssocks[ns] == -1
-					     && errno == EINVAL ? -1 : 1);
+					     && __is_socket_error (errno) ? -1 : 1);
 #endif
 			}
 			if (__builtin_expect (__have_o_nonblock < 0, 0))
@@ -943,7 +943,7 @@ reopen (res_state statp, int *terrno, in
 				if (__have_o_nonblock == 0)
 					__have_o_nonblock
 					  = (EXT(statp).nssocks[ns] == -1
-					     && errno == EINVAL ? -1 : 1);
+					     && __is_socket_error (errno) ? -1 : 1);
 #endif
 			}
 			if (__builtin_expect (__have_o_nonblock < 0, 0))
--- a/socket/Versions
+++ b/socket/Versions
@@ -39,5 +39,6 @@ libc {
   }
   GLIBC_PRIVATE {
     __sendmmsg;
+    __is_socket_error;
   }
 }
--- a/socket/have_sock_cloexec.c
+++ b/socket/have_sock_cloexec.c
@@ -18,9 +18,25 @@
 #include <fcntl.h>
 #include <sys/socket.h>
 #include <kernel-features.h>
+#include <errno.h>
 
 #if defined SOCK_CLOEXEC && !defined __ASSUME_SOCK_CLOEXEC
 int __have_sock_cloexec;
+
+bool
+__is_socket_error (int err)
+{
+  switch (err)
+    {
+    case EINVAL:
+    case ESOCKTNOSUPPORT:
+    case EPROTOTYPE:
+    case EPROTONOSUPPORT:
+      return true;
+    }
+
+  return false;
+}
 #endif
 
 #if defined O_CLOEXEC && !defined __ASSUME_PIPE2
--- a/sunrpc/clnt_udp.c
+++ b/sunrpc/clnt_udp.c
@@ -179,7 +179,7 @@ __libc_clntudp_bufcreate (struct sockadd
 			     IPPROTO_UDP);
 # ifndef __ASSUME_SOCK_CLOEXEC
 	  if (__have_sock_cloexec == 0)
-	    __have_sock_cloexec = *sockp >= 0 || errno != EINVAL ? 1 : -1;
+	    __have_sock_cloexec = *sockp >= 0 || !__is_socket_error (errno) ? 1 : -1;
 # endif
 	}
 #endif

Attachment: signature.asc
Description: This is a digitally signed message part.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]