Message ID | 20200928144556.239160-4-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [1/6] sysvipc: Fix SEM_STAT_ANY kernel argument pass [BZ #26637] | expand |
* Adhemerval Zanella via Libc-alpha: > It avoids regressions on possible future commands that might require > additional libc support. The downside is new commands added by newer > kernels will need further glibc support. Looks okay to me for now. I think eventually we need to rethink what we want to do about these, for the architectures that don't need translation. Thanks, Florian
On 29/09/2020 04:58, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> It avoids regressions on possible future commands that might require >> additional libc support. The downside is new commands added by newer >> kernels will need further glibc support. > > Looks okay to me for now. > > I think eventually we need to rethink what we want to do about these, > for the architectures that don't need translation. We already have __IPC_TIME64 to tell apart the ABI that require it, we might to put the switch only if it is set. However, I am not sure if it is really an improvement, since we will still need to revise it for architecture with __IPC_TIME64 == 1 anyway.
* Adhemerval Zanella: > On 29/09/2020 04:58, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> It avoids regressions on possible future commands that might require >>> additional libc support. The downside is new commands added by newer >>> kernels will need further glibc support. >> >> Looks okay to me for now. >> >> I think eventually we need to rethink what we want to do about these, >> for the architectures that don't need translation. > > We already have __IPC_TIME64 to tell apart the ABI that require it, > we might to put the switch only if it is set. However, I am not sure > if it is really an improvement, since we will still need to revise > it for architecture with __IPC_TIME64 == 1 anyway. It's an improvement for users on architectures which can use this facility. I think we should not artificially hold back architectures with full userspace/kernel alignment just because there are other architectures out there which are more complicated. Thanks, Florian
diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c index a1f24ab242..5a5e7482f2 100644 --- a/sysdeps/unix/sysv/linux/msgctl.c +++ b/sysdeps/unix/sysv/linux/msgctl.c @@ -88,25 +88,45 @@ __msgctl64 (int msqid, int cmd, struct __msqid64_ds *buf) { #if __IPC_TIME64 struct kernel_msqid64_ds ksemid, *arg = NULL; - if (buf != NULL) +#else + msgctl_arg_t *arg; +#endif + + switch (cmd) { - /* This is a Linux extension where kernel returns a 'struct msginfo' - instead. */ - if (cmd == IPC_INFO || cmd == MSG_INFO) - arg = (struct kernel_msqid64_ds *) buf; - else + case IPC_RMID: + arg = NULL; + break; + + case IPC_SET: + case IPC_STAT: + case MSG_STAT: +#if __IPC_TIME64 + if (buf != NULL) { msqid64_to_kmsqid64 (buf, &ksemid); arg = &ksemid; } - } # ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T - if (cmd == IPC_SET) - arg->msg_perm.mode *= 0x10000U; + if (cmd == IPC_SET) + arg->msg_perm.mode *= 0x10000U; # endif #else - msgctl_arg_t *arg = buf; + arg = buf; #endif + break; + + case IPC_INFO: + case MSG_INFO: + /* This is a Linux extension where kernel returns a 'struct msginfo' + instead. */ + arg = (__typeof__ (arg)) buf; + break; + + default: + __set_errno (EINVAL); + return -1; + } int ret = msgctl_syscall (msqid, cmd, arg); if (ret < 0) diff --git a/sysvipc/test-sysvmsg.c b/sysvipc/test-sysvmsg.c index 84efdade5e..ada2881065 100644 --- a/sysvipc/test-sysvmsg.c +++ b/sysvipc/test-sysvmsg.c @@ -24,6 +24,8 @@ #include <sys/ipc.h> #include <sys/msg.h> +#include <test-sysvipc.h> + #include <support/support.h> #include <support/check.h> #include <support/temp_file.h> @@ -86,6 +88,9 @@ do_test (void) FAIL_EXIT1 ("msgget failed (errno=%d)", errno); } + TEST_COMPARE (msgctl (msqid, first_msg_invalid_cmd (), NULL), -1); + TEST_COMPARE (errno, EINVAL); + /* Get message queue kernel information and do some sanity checks. */ struct msqid_ds msginfo; if (msgctl (msqid, IPC_STAT, &msginfo) == -1)