Messages in this thread Patch in this message | | | Date | Thu, 12 Feb 2015 17:32:18 +0000 | From | Catalin Marinas <> | Subject | Re: [PATCH] net: compat: Ignore MSG_CMSG_COMPAT in compat_sys_{send,recv}msg |
| |
On Thu, Feb 12, 2015 at 04:41:24PM +0000, David Miller wrote: > From: Catalin Marinas <catalin.marinas@arm.com> > Date: Thu, 12 Feb 2015 12:28:07 +0000 > > > With commit a7526eb5d06b (net: Unbreak compat_sys_{send,recv}msg), the > > MSG_CMSG_COMPAT flag is blocked at the compat syscall entry points, > > changing the kernel compat behaviour from the one before the commit it > > was trying to fix (1be374a0518a, net: Block MSG_CMSG_COMPAT in > > send(m)msg and recv(m)msg). > > > > On 32-bit kernels (!CONFIG_COMPAT), MSG_CMSG_COMPAT is 0 and the native > > 32-bit sys_sendmsg() allows flag 0x80000000 to be set (it is ignored by > > the kernel). However, on a 64-bit kernel, the compat ABI is different > > with commit a7526eb5d06b. > > > > This patch changes the compat_sys_{send,recv}msg behaviour to the one > > prior to commit 1be374a0518a. > > > > The problem was found running 32-bit LTP (sendmsg01) binary on an arm64 > > kernel. Arguably, LTP should not pass 0xffffffff as flags to sendmsg() > > but the general rule is not to break user ABI (even when the user > > behaviour is not entirely sane). > > > > Fixes: a7526eb5d06b (net: Unbreak compat_sys_{send,recv}msg) > > Cc: Andy Lutomirski <luto@amacapital.net> > > Cc: David S. Miller <davem@davemloft.net> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > I think this is a very poor LTP test.
That I agree.
> Setting MSG_* bits that aren't supported by the protocol in any way > gives undefined semantics. You may get an error, it may be silently > ignored, etc.
From the sendmsg(2) man page, if some bit in the flags is inappropriate for the socket type, it should return -EOPNOTSUPP. But I can't tell whether it refers only to bits which are defined (for other socket types) or just any bit. The test itself checks for -EOPNOTSUPP but it gets -EINVAL instead, hence the failure being reported.
> I'm not applying this, sorry.
What I'm after is consistency between the native 32-bit kernel and the compat layer on 64-bit. On the former, bit 31 is silently ignored, on the latter it reports -EINVAL.
We could as well do something like below but we end up with unnecessary flags check on 32-bit. The question is whether such change would be considered a 32-bit user ABI breakage.
diff --git a/include/linux/socket.h b/include/linux/socket.h index 6e49a14365dc..0b283397ca0a 100644 --- a/include/linux/socket.h +++ b/include/linux/socket.h @@ -272,11 +272,7 @@ struct ucred { #define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file descriptor received through SCM_RIGHTS */ -#if defined(CONFIG_COMPAT) #define MSG_CMSG_COMPAT 0x80000000 /* This message needs 32 bit fixups */ -#else -#define MSG_CMSG_COMPAT 0 /* We never have 32 bit fixups */ -#endif /* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */ -- Catalin
| |