| Message ID | 1363646825-36190-1-git-send-email-petar.jovanovic@rt-rk.com |
|---|---|
| State | New |
| Headers | show |
I am reviewing that for the MIPS part, and I am not really used to the linux-user code, so a review from someone familiar with it would be appreciated. I added Richard Henderson and Blue Swirl in Cc: as alpha and sparc are also affected by this issue. On Mon, Mar 18, 2013 at 11:47:05PM +0100, Petar Jovanovic wrote: > From: Petar Jovanovic <petar.jovanovic@imgtec.com> > > Previous implementation has failed to take into account different value of > SOCK_NONBLOCK on target (MIPS) and host, and existence of SOCK_CLOEXEC. > The same conversion has to be applied both for do_socket and do_socketpair, > so the code has been isolated in a static inline function. > It is defined for MIPS target only. > > enum sock_type in linux-user/socket.h has been extended to include > TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc. > The patch also includes necessary code style changes (tab to spaces) in the > header file in the MIPS #ifdef block touched by the change. > > Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com> > --- > linux-user/socket.h | 172 ++++++++++++++++++++++++++------------------------ > linux-user/syscall.c | 45 ++++++++----- > 2 files changed, 119 insertions(+), 98 deletions(-) > > diff --git a/linux-user/socket.h b/linux-user/socket.h > index 339cae5..e4dfe56 100644 > --- a/linux-user/socket.h > +++ b/linux-user/socket.h > @@ -1,91 +1,101 @@ > > #if defined(TARGET_MIPS) > - // MIPS special values for constants > - > - /* > - * For setsockopt(2) > - * > - * This defines are ABI conformant as far as Linux supports these ... > - */ > - #define TARGET_SOL_SOCKET 0xffff > - > - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */ > - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */ > - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send > - SIGPIPE when they die. */ > - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */ > - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of > - broadcast messages. */ > - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable > - socket to transmit pending data. */ > - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */ > - #if 0 > - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */ > - #endif > - > - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */ > - #define TARGET_SO_STYLE SO_TYPE /* Synonym */ > - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */ > - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */ > - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */ > - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */ > - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */ > - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */ > - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */ > - #define TARGET_SO_ACCEPTCONN 0x1009 > - > - /* linux-specific, might as well be the same as on i386 */ > - #define TARGET_SO_NO_CHECK 11 > - #define TARGET_SO_PRIORITY 12 > - #define TARGET_SO_BSDCOMPAT 14 > + /* MIPS special values for constants */ > + > + /* > + * For setsockopt(2) > + * > + * This defines are ABI conformant as far as Linux supports these ... > + */ > + #define TARGET_SOL_SOCKET 0xffff > + > + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */ > + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */ > + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send > + SIGPIPE when they die. */ > + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */ > + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of > + broadcast messages. */ > + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable > + * socket to transmit pending data. > + */ > + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. > + */ > + #if 0 > + /* To add: Allow local address and port reuse. */ > + #define TARGET_SO_REUSEPORT 0x0200 > + #endif > + > + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */ > + #define TARGET_SO_STYLE SO_TYPE /* Synonym */ > + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */ > + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */ > + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */ > + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */ > + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */ > + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */ > + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */ > + #define TARGET_SO_ACCEPTCONN 0x1009 > > - #define TARGET_SO_PASSCRED 17 > - #define TARGET_SO_PEERCRED 18 > + /* linux-specific, might as well be the same as on i386 */ > + #define TARGET_SO_NO_CHECK 11 > + #define TARGET_SO_PRIORITY 12 > + #define TARGET_SO_BSDCOMPAT 14 > > - /* Security levels - as per NRL IPv6 - don't actually do anything */ > - #define TARGET_SO_SECURITY_AUTHENTICATION 22 > - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23 > - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24 > + #define TARGET_SO_PASSCRED 17 > + #define TARGET_SO_PEERCRED 18 > + > + /* Security levels - as per NRL IPv6 - don't actually do anything */ > + #define TARGET_SO_SECURITY_AUTHENTICATION 22 > + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23 > + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24 > > - #define TARGET_SO_BINDTODEVICE 25 > + #define TARGET_SO_BINDTODEVICE 25 > > - /* Socket filtering */ > - #define TARGET_SO_ATTACH_FILTER 26 > - #define TARGET_SO_DETACH_FILTER 27 > + /* Socket filtering */ > + #define TARGET_SO_ATTACH_FILTER 26 > + #define TARGET_SO_DETACH_FILTER 27 > > - #define TARGET_SO_PEERNAME 28 > - #define TARGET_SO_TIMESTAMP 29 > - #define SCM_TIMESTAMP SO_TIMESTAMP > - > - #define TARGET_SO_PEERSEC 30 > - #define TARGET_SO_SNDBUFFORCE 31 > - #define TARGET_SO_RCVBUFFORCE 33 > - > - /** sock_type - Socket types > - * > - * Please notice that for binary compat reasons MIPS has to > - * override the enum sock_type in include/linux/net.h, so > - * we define ARCH_HAS_SOCKET_TYPES here. > - * > - * @SOCK_DGRAM - datagram (conn.less) socket > - * @SOCK_STREAM - stream (connection) socket > - * @SOCK_RAW - raw socket > - * @SOCK_RDM - reliably-delivered message > - * @SOCK_SEQPACKET - sequential packet socket > - * @SOCK_PACKET - linux specific way of getting packets at the dev level. > - * For writing rarp and other similar things on the user level. > - */ > - enum sock_type { > - TARGET_SOCK_DGRAM = 1, > - TARGET_SOCK_STREAM = 2, > - TARGET_SOCK_RAW = 3, > - TARGET_SOCK_RDM = 4, > - TARGET_SOCK_SEQPACKET = 5, > - TARGET_SOCK_DCCP = 6, > - TARGET_SOCK_PACKET = 10, > - }; > - > - #define TARGET_SOCK_MAX (SOCK_PACKET + 1) > + #define TARGET_SO_PEERNAME 28 > + #define TARGET_SO_TIMESTAMP 29 > + #define SCM_TIMESTAMP SO_TIMESTAMP > + > + #define TARGET_SO_PEERSEC 30 > + #define TARGET_SO_SNDBUFFORCE 31 > + #define TARGET_SO_RCVBUFFORCE 33 > + > + /** sock_type - Socket types > + * > + * Please notice that for binary compat reasons MIPS has to > + * override the enum sock_type in include/linux/net.h, so > + * we define ARCH_HAS_SOCKET_TYPES here. > + * Not related to your changes, but note that ARCH_HAS_SOCKET_TYPES is not defined anywhere contrary to what is said in the comment. It might be a good idea to revive it, as at least alpha and sparc also have the issue. > + * @SOCK_DGRAM - datagram (conn.less) socket > + * @SOCK_STREAM - stream (connection) socket > + * @SOCK_RAW - raw socket > + * @SOCK_RDM - reliably-delivered message > + * @SOCK_SEQPACKET - sequential packet socket > + * @SOCK_DCCP - Datagram Congestion Control Protocol socket > + * @SOCK_PACKET - linux specific way of getting packets at the dev level. > + * For writing rarp and other similar things on the user > + * level. > + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag. > + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag. > + */ > + enum sock_type { > + TARGET_SOCK_DGRAM = 1, > + TARGET_SOCK_STREAM = 2, > + TARGET_SOCK_RAW = 3, > + TARGET_SOCK_RDM = 4, > + TARGET_SOCK_SEQPACKET = 5, > + TARGET_SOCK_DCCP = 6, > + TARGET_SOCK_PACKET = 10, > + TARGET_SOCK_CLOEXEC = 02000000, > + TARGET_SOCK_NONBLOCK = 0200, > + }; > + > + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1) > + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */ > > #elif defined(TARGET_ALPHA) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index ee82a2d..0a9e6db 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr, > free(vec); > } > > -/* do_socket() Must return target values and target errnos. */ > -static abi_long do_socket(int domain, int type, int protocol) > -{ > #if defined(TARGET_MIPS) > - switch(type) { > +static inline void target_to_host_sock_type(int *type) > +{ > + int host_type = 0; > + int target_type = *type; > + > + switch (target_type & TARGET_SOCK_TYPE_MASK) { > case TARGET_SOCK_DGRAM: > - type = SOCK_DGRAM; > + host_type = SOCK_DGRAM; > break; > case TARGET_SOCK_STREAM: > - type = SOCK_STREAM; > + host_type = SOCK_STREAM; > break; > - case TARGET_SOCK_RAW: > - type = SOCK_RAW; > - break; > - case TARGET_SOCK_RDM: > - type = SOCK_RDM; > - break; > - case TARGET_SOCK_SEQPACKET: > - type = SOCK_SEQPACKET; > - break; > - case TARGET_SOCK_PACKET: > - type = SOCK_PACKET; > + default: > + host_type = target_type & TARGET_SOCK_TYPE_MASK; > break; > } I am not sure we want to really copy the type without more checking than the mask: a new value still within the mask limit could be added differently depending on the architecture. > + if (target_type & TARGET_SOCK_CLOEXEC) { > + host_type |= SOCK_CLOEXEC; > + } > + if (target_type & TARGET_SOCK_NONBLOCK) { > + host_type |= SOCK_NONBLOCK; > + } > + *type = host_type; Also I think the values should be checked in all cases returning -EINVAL if not supported. On other architecture where no translation is done, this is done by the host kernel. With the code above, unsupported arguments are simply ignored. > +} > +#endif > + > +/* do_socket() Must return target values and target errnos. */ > +static abi_long do_socket(int domain, int type, int protocol) > +{ > +#if defined(TARGET_MIPS) > + target_to_host_sock_type(&type); > #endif As said above this could probably be conditional on ARCH_HAS_SOCKET_TYPES to make it more generic. > if (domain == PF_NETLINK) > return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */ > @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol, > int tab[2]; > abi_long ret; > > +#if defined(TARGET_MIPS) > + target_to_host_sock_type(&type); > +#endif > ret = get_errno(socketpair(domain, type, protocol, tab)); > if (!is_error(ret)) { > if (put_user_s32(tab[0], target_tab_addr) > -- > 1.7.9.5 > >
diff --git a/linux-user/socket.h b/linux-user/socket.h index 339cae5..e4dfe56 100644 --- a/linux-user/socket.h +++ b/linux-user/socket.h @@ -1,91 +1,101 @@ #if defined(TARGET_MIPS) - // MIPS special values for constants - - /* - * For setsockopt(2) - * - * This defines are ABI conformant as far as Linux supports these ... - */ - #define TARGET_SOL_SOCKET 0xffff - - #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */ - #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */ - #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send - SIGPIPE when they die. */ - #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */ - #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of - broadcast messages. */ - #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable - socket to transmit pending data. */ - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. */ - #if 0 - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address and port reuse. */ - #endif - - #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */ - #define TARGET_SO_STYLE SO_TYPE /* Synonym */ - #define TARGET_SO_ERROR 0x1007 /* get error status and clear */ - #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */ - #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */ - #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */ - #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */ - #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */ - #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */ - #define TARGET_SO_ACCEPTCONN 0x1009 - - /* linux-specific, might as well be the same as on i386 */ - #define TARGET_SO_NO_CHECK 11 - #define TARGET_SO_PRIORITY 12 - #define TARGET_SO_BSDCOMPAT 14 + /* MIPS special values for constants */ + + /* + * For setsockopt(2) + * + * This defines are ABI conformant as far as Linux supports these ... + */ + #define TARGET_SOL_SOCKET 0xffff + + #define TARGET_SO_DEBUG 0x0001 /* Record debugging information. */ + #define TARGET_SO_REUSEADDR 0x0004 /* Allow reuse of local addresses. */ + #define TARGET_SO_KEEPALIVE 0x0008 /* Keep connections alive and send + SIGPIPE when they die. */ + #define TARGET_SO_DONTROUTE 0x0010 /* Don't do local routing. */ + #define TARGET_SO_BROADCAST 0x0020 /* Allow transmission of + broadcast messages. */ + #define TARGET_SO_LINGER 0x0080 /* Block on close of a reliable + * socket to transmit pending data. + */ + #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data in-band. + */ + #if 0 + /* To add: Allow local address and port reuse. */ + #define TARGET_SO_REUSEPORT 0x0200 + #endif + + #define TARGET_SO_TYPE 0x1008 /* Compatible name for SO_STYLE. */ + #define TARGET_SO_STYLE SO_TYPE /* Synonym */ + #define TARGET_SO_ERROR 0x1007 /* get error status and clear */ + #define TARGET_SO_SNDBUF 0x1001 /* Send buffer size. */ + #define TARGET_SO_RCVBUF 0x1002 /* Receive buffer. */ + #define TARGET_SO_SNDLOWAT 0x1003 /* send low-water mark */ + #define TARGET_SO_RCVLOWAT 0x1004 /* receive low-water mark */ + #define TARGET_SO_SNDTIMEO 0x1005 /* send timeout */ + #define TARGET_SO_RCVTIMEO 0x1006 /* receive timeout */ + #define TARGET_SO_ACCEPTCONN 0x1009 - #define TARGET_SO_PASSCRED 17 - #define TARGET_SO_PEERCRED 18 + /* linux-specific, might as well be the same as on i386 */ + #define TARGET_SO_NO_CHECK 11 + #define TARGET_SO_PRIORITY 12 + #define TARGET_SO_BSDCOMPAT 14 - /* Security levels - as per NRL IPv6 - don't actually do anything */ - #define TARGET_SO_SECURITY_AUTHENTICATION 22 - #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23 - #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24 + #define TARGET_SO_PASSCRED 17 + #define TARGET_SO_PEERCRED 18 + + /* Security levels - as per NRL IPv6 - don't actually do anything */ + #define TARGET_SO_SECURITY_AUTHENTICATION 22 + #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23 + #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK 24 - #define TARGET_SO_BINDTODEVICE 25 + #define TARGET_SO_BINDTODEVICE 25 - /* Socket filtering */ - #define TARGET_SO_ATTACH_FILTER 26 - #define TARGET_SO_DETACH_FILTER 27 + /* Socket filtering */ + #define TARGET_SO_ATTACH_FILTER 26 + #define TARGET_SO_DETACH_FILTER 27 - #define TARGET_SO_PEERNAME 28 - #define TARGET_SO_TIMESTAMP 29 - #define SCM_TIMESTAMP SO_TIMESTAMP - - #define TARGET_SO_PEERSEC 30 - #define TARGET_SO_SNDBUFFORCE 31 - #define TARGET_SO_RCVBUFFORCE 33 - - /** sock_type - Socket types - * - * Please notice that for binary compat reasons MIPS has to - * override the enum sock_type in include/linux/net.h, so - * we define ARCH_HAS_SOCKET_TYPES here. - * - * @SOCK_DGRAM - datagram (conn.less) socket - * @SOCK_STREAM - stream (connection) socket - * @SOCK_RAW - raw socket - * @SOCK_RDM - reliably-delivered message - * @SOCK_SEQPACKET - sequential packet socket - * @SOCK_PACKET - linux specific way of getting packets at the dev level. - * For writing rarp and other similar things on the user level. - */ - enum sock_type { - TARGET_SOCK_DGRAM = 1, - TARGET_SOCK_STREAM = 2, - TARGET_SOCK_RAW = 3, - TARGET_SOCK_RDM = 4, - TARGET_SOCK_SEQPACKET = 5, - TARGET_SOCK_DCCP = 6, - TARGET_SOCK_PACKET = 10, - }; - - #define TARGET_SOCK_MAX (SOCK_PACKET + 1) + #define TARGET_SO_PEERNAME 28 + #define TARGET_SO_TIMESTAMP 29 + #define SCM_TIMESTAMP SO_TIMESTAMP + + #define TARGET_SO_PEERSEC 30 + #define TARGET_SO_SNDBUFFORCE 31 + #define TARGET_SO_RCVBUFFORCE 33 + + /** sock_type - Socket types + * + * Please notice that for binary compat reasons MIPS has to + * override the enum sock_type in include/linux/net.h, so + * we define ARCH_HAS_SOCKET_TYPES here. + * + * @SOCK_DGRAM - datagram (conn.less) socket + * @SOCK_STREAM - stream (connection) socket + * @SOCK_RAW - raw socket + * @SOCK_RDM - reliably-delivered message + * @SOCK_SEQPACKET - sequential packet socket + * @SOCK_DCCP - Datagram Congestion Control Protocol socket + * @SOCK_PACKET - linux specific way of getting packets at the dev level. + * For writing rarp and other similar things on the user + * level. + * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag. + * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag. + */ + enum sock_type { + TARGET_SOCK_DGRAM = 1, + TARGET_SOCK_STREAM = 2, + TARGET_SOCK_RAW = 3, + TARGET_SOCK_RDM = 4, + TARGET_SOCK_SEQPACKET = 5, + TARGET_SOCK_DCCP = 6, + TARGET_SOCK_PACKET = 10, + TARGET_SOCK_CLOEXEC = 02000000, + TARGET_SOCK_NONBLOCK = 0200, + }; + + #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1) + #define TARGET_SOCK_TYPE_MASK 0xf /* Covers up to TARGET_SOCK_MAX-1. */ #elif defined(TARGET_ALPHA) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ee82a2d..0a9e6db 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1859,30 +1859,38 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr, free(vec); } -/* do_socket() Must return target values and target errnos. */ -static abi_long do_socket(int domain, int type, int protocol) -{ #if defined(TARGET_MIPS) - switch(type) { +static inline void target_to_host_sock_type(int *type) +{ + int host_type = 0; + int target_type = *type; + + switch (target_type & TARGET_SOCK_TYPE_MASK) { case TARGET_SOCK_DGRAM: - type = SOCK_DGRAM; + host_type = SOCK_DGRAM; break; case TARGET_SOCK_STREAM: - type = SOCK_STREAM; + host_type = SOCK_STREAM; break; - case TARGET_SOCK_RAW: - type = SOCK_RAW; - break; - case TARGET_SOCK_RDM: - type = SOCK_RDM; - break; - case TARGET_SOCK_SEQPACKET: - type = SOCK_SEQPACKET; - break; - case TARGET_SOCK_PACKET: - type = SOCK_PACKET; + default: + host_type = target_type & TARGET_SOCK_TYPE_MASK; break; } + if (target_type & TARGET_SOCK_CLOEXEC) { + host_type |= SOCK_CLOEXEC; + } + if (target_type & TARGET_SOCK_NONBLOCK) { + host_type |= SOCK_NONBLOCK; + } + *type = host_type; +} +#endif + +/* do_socket() Must return target values and target errnos. */ +static abi_long do_socket(int domain, int type, int protocol) +{ +#if defined(TARGET_MIPS) + target_to_host_sock_type(&type); #endif if (domain == PF_NETLINK) return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */ @@ -2116,6 +2124,9 @@ static abi_long do_socketpair(int domain, int type, int protocol, int tab[2]; abi_long ret; +#if defined(TARGET_MIPS) + target_to_host_sock_type(&type); +#endif ret = get_errno(socketpair(domain, type, protocol, tab)); if (!is_error(ret)) { if (put_user_s32(tab[0], target_tab_addr)