| Message ID | 20200303192725.16847-1-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux: Clear mode_t padding bits (BZ#25623) | expand |
* Adhemerval Zanella: > diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c > index 27879e76cd..48801a3fba 100644 > --- a/sysdeps/unix/sysv/linux/msgctl.c > +++ b/sysdeps/unix/sysv/linux/msgctl.c > +#else > + /* Old Linux kernel versions might not clear the mode padding. */ > + if (sizeof ((struct msqid_ds){0}.msg_perm.mode > + != sizeof ((struct __old_ipc_perm){0}.mode))) > + buf->msg_perm.mode &= 0xFFFF; > +#endif The condition always evaluates to true becaduse the first sizeof is applied to the result of the != operator. Thanks, Florian
On 03/03/2020 18:51, Florian Weimer wrote: > * Adhemerval Zanella: > >> diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c >> index 27879e76cd..48801a3fba 100644 >> --- a/sysdeps/unix/sysv/linux/msgctl.c >> +++ b/sysdeps/unix/sysv/linux/msgctl.c > >> +#else >> + /* Old Linux kernel versions might not clear the mode padding. */ >> + if (sizeof ((struct msqid_ds){0}.msg_perm.mode >> + != sizeof ((struct __old_ipc_perm){0}.mode))) >> + buf->msg_perm.mode &= 0xFFFF; >> +#endif > > The condition always evaluates to true becaduse the first sizeof is > applied to the result of the != operator. Sigh, indeed (I should have verified if x86_64 resulting code would be constant). Is is ok with the suppose fix (below)? if (sizeof ((struct msqid_ds){0}.msg_perm.mode) != sizeof ((struct __old_ipc_perm){0}.mode)) buf->msg_perm.mode &= 0xFFFF;
* Adhemerval Zanella: > On 03/03/2020 18:51, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c >>> index 27879e76cd..48801a3fba 100644 >>> --- a/sysdeps/unix/sysv/linux/msgctl.c >>> +++ b/sysdeps/unix/sysv/linux/msgctl.c >> >>> +#else >>> + /* Old Linux kernel versions might not clear the mode padding. */ >>> + if (sizeof ((struct msqid_ds){0}.msg_perm.mode >>> + != sizeof ((struct __old_ipc_perm){0}.mode))) >>> + buf->msg_perm.mode &= 0xFFFF; >>> +#endif >> >> The condition always evaluates to true becaduse the first sizeof is >> applied to the result of the != operator. > > Sigh, indeed (I should have verified if x86_64 resulting code would be > constant). Is is ok with the suppose fix (below)? > > if (sizeof ((struct msqid_ds){0}.msg_perm.mode) > != sizeof ((struct __old_ipc_perm){0}.mode)) > buf->msg_perm.mode &= 0xFFFF; That looks better. Is there case where the sizes are the same and 32 bits? Thanks, Florian
On Mär 03 2020, Adhemerval Zanella wrote: > On 03/03/2020 18:51, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c >>> index 27879e76cd..48801a3fba 100644 >>> --- a/sysdeps/unix/sysv/linux/msgctl.c >>> +++ b/sysdeps/unix/sysv/linux/msgctl.c >> >>> +#else >>> + /* Old Linux kernel versions might not clear the mode padding. */ >>> + if (sizeof ((struct msqid_ds){0}.msg_perm.mode >>> + != sizeof ((struct __old_ipc_perm){0}.mode))) >>> + buf->msg_perm.mode &= 0xFFFF; >>> +#endif >> >> The condition always evaluates to true becaduse the first sizeof is >> applied to the result of the != operator. > > Sigh, indeed (I should have verified if x86_64 resulting code would be > constant). Is is ok with the suppose fix (below)? > > if (sizeof ((struct msqid_ds){0}.msg_perm.mode) > != sizeof ((struct __old_ipc_perm){0}.mode)) > buf->msg_perm.mode &= 0xFFFF; I don't think __old_ipc_perm is the right type to check since that is for use by the !__IPC_64 syscall. What we really need to check here is __kernel_mode_t vs. mode_t. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
diff --git a/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h deleted file mode 100644 index 07a1c3a7d3..0000000000 --- a/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h +++ /dev/null @@ -1,21 +0,0 @@ -/* Old SysV permission definition for Linux. AArch64 version. - Copyright (C) 2016-2020 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#include <sys/ipc.h> /* For __key_t */ - -#define __IPC_64 0x0 diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c index 27879e76cd..48801a3fba 100644 --- a/sysdeps/unix/sysv/linux/msgctl.c +++ b/sysdeps/unix/sysv/linux/msgctl.c @@ -61,7 +61,6 @@ __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) int ret = msgctl_syscall (msqid, cmd, buf); -#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T if (ret >= 0) { switch (cmd) @@ -69,10 +68,16 @@ __new_msgctl (int msqid, int cmd, struct msqid_ds *buf) case IPC_STAT: case MSG_STAT: case MSG_STAT_ANY: +#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T buf->msg_perm.mode >>= 16; +#else + /* Old Linux kernel versions might not clear the mode padding. */ + if (sizeof ((struct msqid_ds){0}.msg_perm.mode + != sizeof ((struct __old_ipc_perm){0}.mode))) + buf->msg_perm.mode &= 0xFFFF; +#endif } } -#endif return ret; } diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c index 0c3eb0932f..580a72febe 100644 --- a/sysdeps/unix/sysv/linux/semctl.c +++ b/sysdeps/unix/sysv/linux/semctl.c @@ -92,7 +92,6 @@ __new_semctl (int semid, int semnum, int cmd, ...) int ret = semctl_syscall (semid, semnum, cmd, arg); -#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T if (ret >= 0) { switch (cmd) @@ -100,10 +99,16 @@ __new_semctl (int semid, int semnum, int cmd, ...) case IPC_STAT: case SEM_STAT: case SEM_STAT_ANY: +#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T arg.buf->sem_perm.mode >>= 16; +#else + /* Old Linux kernel versions might not clear the mode padding. */ + if (sizeof ((struct semid_ds){0}.sem_perm.mode + != sizeof ((struct __old_ipc_perm){0}.mode))) + arg.buf->sem_perm.mode &= 0xFFFF; +#endif } } -#endif return ret; } diff --git a/sysdeps/unix/sysv/linux/shmctl.c b/sysdeps/unix/sysv/linux/shmctl.c index 39fa861e17..9a44ad6920 100644 --- a/sysdeps/unix/sysv/linux/shmctl.c +++ b/sysdeps/unix/sysv/linux/shmctl.c @@ -63,7 +63,6 @@ __new_shmctl (int shmid, int cmd, struct shmid_ds *buf) int ret = shmctl_syscall (shmid, cmd, buf); -#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T if (ret >= 0) { switch (cmd) @@ -71,10 +70,16 @@ __new_shmctl (int shmid, int cmd, struct shmid_ds *buf) case IPC_STAT: case SHM_STAT: case SHM_STAT_ANY: +#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T buf->shm_perm.mode >>= 16; +#else + /* Old Linux kernel versions might not clear the mode padding. */ + if (sizeof ((struct shmid_ds){0}.shm_perm.mode) + != sizeof ((struct __old_ipc_perm){0}.mode)) + buf->shm_perm.mode &= 0xFFFF; +#endif } } -#endif return ret; }