| Message ID | 20180420130346.3178914-1-arnd@arndb.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: ipc: fix x32 version of shmid64_ds and msqid64_ds | expand |
Hi Arnd, One comment here: > +#if !defined(__x86_64__) || !defined(__ilp32__) > #include <asm-generic/msgbuf.h> > +#else I understand there's some progress having Clang compile the kernel. Clang treats __ILP32__ and friends differently than GCC. I believe ILP32 shows up just about everywhere there are 32-bit ints, longs and pointers. You might find it on Aarch64 or you might find it on MIPS64 when using Clang. I think that means this may be a little suspicious: > +#if !defined(__x86_64__) || !defined(__ilp32__) I kind of felt LLVM was wandering away from the x32 ABI, but the LLVM devs insisted they were within their purview. Also see https://lists.llvm.org/pipermail/cfe-dev/2015-December/046300.html. Sorry about the top-post. I just wanted to pick out that one piece. Jeff On Fri, Apr 20, 2018 at 9:03 AM, Arnd Bergmann <arnd@arndb.de> wrote: > A bugfix broke the x32 shmid64_ds and msqid64_ds data structure layout > (as seen from user space) a few years ago: Originally, __BITS_PER_LONG > was defined as 64 on x32, so we did not have padding after the 64-bit > __kernel_time_t fields, After __BITS_PER_LONG got changed to 32, > applications would observe extra padding. > > In other parts of the uapi headers we seem to have a mix of those > expecting either 32 or 64 on x32 applications, so we can't easily revert > the path that broke these two structures. > > Instead, this patch decouples x32 from the other architectures and moves > it back into arch specific headers, partially reverting the even older > commit 73a2d096fdf2 ("x86: remove all now-duplicate header files"). > > It's not clear whether this ever made any difference, since at least > glibc carries its own (correct) copy of both of these header files, > so possibly no application has ever observed the definitions here. > > There are other UAPI interfaces that depend on __BITS_PER_LONG and > that might suffer from the same problem on x32, but I have not tried to > analyse them in enough detail to be sure. If anyone still cares about x32, > that may be a useful thing to do. > > Fixes: f4b4aae18288 ("x86/headers/uapi: Fix __BITS_PER_LONG value for x32 builds") > Cc: stable@vger.kernel.org > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > This came out of the y2038 ipc syscall series but can be applied > and backported independently. > --- > arch/x86/include/uapi/asm/msgbuf.h | 29 +++++++++++++++++++++++++++ > arch/x86/include/uapi/asm/shmbuf.h | 40 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 69 insertions(+) > > diff --git a/arch/x86/include/uapi/asm/msgbuf.h b/arch/x86/include/uapi/asm/msgbuf.h > index 809134c644a6..5f1604961e6d 100644 > --- a/arch/x86/include/uapi/asm/msgbuf.h > +++ b/arch/x86/include/uapi/asm/msgbuf.h > @@ -1 +1,30 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +#ifndef __ASM_X64_MSGBUF_H > +#define __ASM_X64_MSGBUF_H > + > +#if !defined(__x86_64__) || !defined(__ilp32__) > #include <asm-generic/msgbuf.h> > +#else > +/* > + * The msqid64_ds structure for x86 architecture with x32 ABI. > + * > + * On x86-32 and x86-64 we can just use the generic definition, but > + * x32 uses the same binary layout as x86_64, which is differnet > + * from other 32-bit architectures. > + */ > + > +struct msqid64_ds { > + struct ipc64_perm msg_perm; > + __kernel_time_t msg_stime; /* last msgsnd time */ > + __kernel_time_t msg_rtime; /* last msgrcv time */ > + __kernel_time_t msg_ctime; /* last change time */ > + __kernel_ulong_t msg_cbytes; /* current number of bytes on queue */ > + __kernel_ulong_t msg_qnum; /* number of messages in queue */ > + __kernel_ulong_t msg_qbytes; /* max number of bytes on queue */ > + __kernel_pid_t msg_lspid; /* pid of last msgsnd */ > + __kernel_pid_t msg_lrpid; /* last receive pid */ > + __kernel_ulong_t __unused4; > + __kernel_ulong_t __unused5; > +}; > + > +#endif /* __ASM_GENERIC_MSGBUF_H */ > diff --git a/arch/x86/include/uapi/asm/shmbuf.h b/arch/x86/include/uapi/asm/shmbuf.h > index 83c05fc2de38..cdd7eec878fa 100644 > --- a/arch/x86/include/uapi/asm/shmbuf.h > +++ b/arch/x86/include/uapi/asm/shmbuf.h > @@ -1 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +#ifndef __ASM_X86_SHMBUF_H > +#define __ASM_X86_SHMBUF_H > + > +#if !defined(__x86_64__) || !defined(__ilp32__) > #include <asm-generic/shmbuf.h> > +#else > +/* > + * The shmid64_ds structure for x86 architecture with x32 ABI. > + * > + * On x86-32 and x86-64 we can just use the generic definition, but > + * x32 uses the same binary layout as x86_64, which is differnet > + * from other 32-bit architectures. > + */ > + > +struct shmid64_ds { > + struct ipc64_perm shm_perm; /* operation perms */ > + size_t shm_segsz; /* size of segment (bytes) */ > + __kernel_time_t shm_atime; /* last attach time */ > + __kernel_time_t shm_dtime; /* last detach time */ > + __kernel_time_t shm_ctime; /* last change time */ > + __kernel_pid_t shm_cpid; /* pid of creator */ > + __kernel_pid_t shm_lpid; /* pid of last operator */ > + __kernel_ulong_t shm_nattch; /* no. of current attaches */ > + __kernel_ulong_t __unused4; > + __kernel_ulong_t __unused5; > +}; > + > +struct shminfo64 { > + __kernel_ulong_t shmmax; > + __kernel_ulong_t shmmin; > + __kernel_ulong_t shmmni; > + __kernel_ulong_t shmseg; > + __kernel_ulong_t shmall; > + __kernel_ulong_t __unused1; > + __kernel_ulong_t __unused2; > + __kernel_ulong_t __unused3; > + __kernel_ulong_t __unused4; > +}; > + > +#endif /* __ASM_X86_SHMBUF_H */
On Fri, Apr 20, 2018 at 3:53 PM, Jeffrey Walton <noloader@gmail.com> wrote: >> +#if !defined(__x86_64__) || !defined(__ilp32__) >> #include <asm-generic/msgbuf.h> >> +#else > > I understand there's some progress having Clang compile the kernel. > Clang treats __ILP32__ and friends differently than GCC. I believe > ILP32 shows up just about everywhere there are 32-bit ints, longs and > pointers. You might find it on Aarch64 or you might find it on MIPS64 > when using Clang. > > I think that means this may be a little suspicious: > > > +#if !defined(__x86_64__) || !defined(__ilp32__) > > I kind of felt LLVM was wandering away from the x32 ABI, but the LLVM > devs insisted they were within their purview. Also see > https://lists.llvm.org/pipermail/cfe-dev/2015-December/046300.html. > > Sorry about the top-post. I just wanted to pick out that one piece. It seems I made a typo and it needs to be __ILP32__ rather than __ilp32__ (corrected that locally, will resend once we have resolved this). Aside from that, the #if check seems to be correct to me: this is an x86-specific header, so it won't ever be seen on other architectures. On x86-32, __x86_64__ isn't set, so we don't care about whether __ilp32__ is set or not, and on x86-64 (lp64), __ilp32__ is never set, so we still get the asm-generic header. Arnd
On Sun, Apr 22, 2018 at 2:38 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Apr 20, 2018 at 7:38 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> On Fri, Apr 20, 2018 at 3:53 PM, Jeffrey Walton <noloader@gmail.com> wrote: > > Glibc has correct header files for system calls. I have a very old > program to check if Linux kernel header files are correct for user > space: > > https://github.com/hjl-tools/linux-header > > It needs update to check uapi. Simply running 'make' on a regular distro shows this output: --- kernel.x32.out 2018-04-22 22:10:16.053432423 +0200 +++ glibc.x32.out 2018-04-22 22:10:16.073432838 +0200 @@ -10,9 +10,9 @@ size of daddr_t: 4 size of __ipc_pid_t: 4 size of struct ipc_perm: 48 size of mqd_t: 4 -size of struct msqid_ds: 144 +size of struct msqid_ds: 120 size of struct semid_ds: 104 -size of struct shmid_ds: 136 +size of struct shmid_ds: 112 size of struct shminfo: 72 size of struct timeval: 16 size of struct timespec: 16 @@ -22,8 +22,8 @@ size of struct mq_attr: 64 size of struct rlimit: 16 size of struct rusage: 144 size of struct stat: 144 -size of struct statfs: 64 -size of struct statfs64: 88 +size of struct statfs: 120 +size of struct statfs64: 120 size of struct timex: 208 size of struct msginfo: 32 size of struct msgbuf: 16 This seems plausible, the statfs structure clearly has the same problem as msqid_ds/shmid_ds based on its usage of '__statfs_word' which is now defined as '__u32' rather than '__kernel_long_t'. It should be trivial to override __statfs_word from arch/x86/include/uapi/asm/statfs.h I've checked the other uses of __BITS_PER_LONG in the uapi headers now, and all the others are either not relevant for x32 (either definition is fine) or it has to be __BITS_PER_LONG=32. Arnd
diff --git a/arch/x86/include/uapi/asm/msgbuf.h b/arch/x86/include/uapi/asm/msgbuf.h index 809134c644a6..5f1604961e6d 100644 --- a/arch/x86/include/uapi/asm/msgbuf.h +++ b/arch/x86/include/uapi/asm/msgbuf.h @@ -1 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __ASM_X64_MSGBUF_H +#define __ASM_X64_MSGBUF_H + +#if !defined(__x86_64__) || !defined(__ilp32__) #include <asm-generic/msgbuf.h> +#else +/* + * The msqid64_ds structure for x86 architecture with x32 ABI. + * + * On x86-32 and x86-64 we can just use the generic definition, but + * x32 uses the same binary layout as x86_64, which is differnet + * from other 32-bit architectures. + */ + +struct msqid64_ds { + struct ipc64_perm msg_perm; + __kernel_time_t msg_stime; /* last msgsnd time */ + __kernel_time_t msg_rtime; /* last msgrcv time */ + __kernel_time_t msg_ctime; /* last change time */ + __kernel_ulong_t msg_cbytes; /* current number of bytes on queue */ + __kernel_ulong_t msg_qnum; /* number of messages in queue */ + __kernel_ulong_t msg_qbytes; /* max number of bytes on queue */ + __kernel_pid_t msg_lspid; /* pid of last msgsnd */ + __kernel_pid_t msg_lrpid; /* last receive pid */ + __kernel_ulong_t __unused4; + __kernel_ulong_t __unused5; +}; + +#endif /* __ASM_GENERIC_MSGBUF_H */ diff --git a/arch/x86/include/uapi/asm/shmbuf.h b/arch/x86/include/uapi/asm/shmbuf.h index 83c05fc2de38..cdd7eec878fa 100644 --- a/arch/x86/include/uapi/asm/shmbuf.h +++ b/arch/x86/include/uapi/asm/shmbuf.h @@ -1 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __ASM_X86_SHMBUF_H +#define __ASM_X86_SHMBUF_H + +#if !defined(__x86_64__) || !defined(__ilp32__) #include <asm-generic/shmbuf.h> +#else +/* + * The shmid64_ds structure for x86 architecture with x32 ABI. + * + * On x86-32 and x86-64 we can just use the generic definition, but + * x32 uses the same binary layout as x86_64, which is differnet + * from other 32-bit architectures. + */ + +struct shmid64_ds { + struct ipc64_perm shm_perm; /* operation perms */ + size_t shm_segsz; /* size of segment (bytes) */ + __kernel_time_t shm_atime; /* last attach time */ + __kernel_time_t shm_dtime; /* last detach time */ + __kernel_time_t shm_ctime; /* last change time */ + __kernel_pid_t shm_cpid; /* pid of creator */ + __kernel_pid_t shm_lpid; /* pid of last operator */ + __kernel_ulong_t shm_nattch; /* no. of current attaches */ + __kernel_ulong_t __unused4; + __kernel_ulong_t __unused5; +}; + +struct shminfo64 { + __kernel_ulong_t shmmax; + __kernel_ulong_t shmmin; + __kernel_ulong_t shmmni; + __kernel_ulong_t shmseg; + __kernel_ulong_t shmall; + __kernel_ulong_t __unused1; + __kernel_ulong_t __unused2; + __kernel_ulong_t __unused3; + __kernel_ulong_t __unused4; +}; + +#endif /* __ASM_X86_SHMBUF_H */
A bugfix broke the x32 shmid64_ds and msqid64_ds data structure layout (as seen from user space) a few years ago: Originally, __BITS_PER_LONG was defined as 64 on x32, so we did not have padding after the 64-bit __kernel_time_t fields, After __BITS_PER_LONG got changed to 32, applications would observe extra padding. In other parts of the uapi headers we seem to have a mix of those expecting either 32 or 64 on x32 applications, so we can't easily revert the path that broke these two structures. Instead, this patch decouples x32 from the other architectures and moves it back into arch specific headers, partially reverting the even older commit 73a2d096fdf2 ("x86: remove all now-duplicate header files"). It's not clear whether this ever made any difference, since at least glibc carries its own (correct) copy of both of these header files, so possibly no application has ever observed the definitions here. There are other UAPI interfaces that depend on __BITS_PER_LONG and that might suffer from the same problem on x32, but I have not tried to analyse them in enough detail to be sure. If anyone still cares about x32, that may be a useful thing to do. Fixes: f4b4aae18288 ("x86/headers/uapi: Fix __BITS_PER_LONG value for x32 builds") Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- This came out of the y2038 ipc syscall series but can be applied and backported independently. --- arch/x86/include/uapi/asm/msgbuf.h | 29 +++++++++++++++++++++++++++ arch/x86/include/uapi/asm/shmbuf.h | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) -- 2.9.0