From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH] Linux: Support __IPC_64 in sysvctl *ctl command arguments
Date: Thu, 10 Nov 2022 09:27:45 -0300 [thread overview]
Message-ID: <ebe416fb-18b9-96ca-bfbf-84aff7a5daeb@linaro.org> (raw)
In-Reply-To: <87bkphij87.fsf@oldenburg.str.redhat.com>
On 08/11/22 12:00, Florian Weimer via Libc-alpha wrote:
> Old applications pass __IPC_64 as part of the command argument because
> old glibc did not check for unknown commands, and passed through the
> arguments directly to the kernel, without adding __IPC_64.
> Applications need to continue doing that for old glibc compatibility,
> so this commit enables this approach in current glibc.
>
> For msgctl and shmctl, if no translation is required, make
> direct system calls, as we did before the time64 changes. If
> translation is required, mask __IPC_64 from the command argument.
>
> For semctl, the union-in-vararg argument handling means that
> translation is needed on all architectures.
>
> Tested on i686-linux-gnu, powerpc64le-linux-gnu, x86_64-linux-gnu.
> Built with build-many-glibcs.py.
Sigh, another internal interface that bleed to userpace. The fix sounds
reasonable to compatibility, just a typo below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> ---
> sysdeps/unix/sysv/linux/ipc_priv.h | 6 ++++++
> sysdeps/unix/sysv/linux/msgctl.c | 38 +++++++++++++++++++++++++-------------
> sysdeps/unix/sysv/linux/semctl.c | 7 +++++++
> sysdeps/unix/sysv/linux/shmctl.c | 38 +++++++++++++++++++++++++-------------
> 4 files changed, 63 insertions(+), 26 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h
> index 87893a6757..2f50c31a8e 100644
> --- a/sysdeps/unix/sysv/linux/ipc_priv.h
> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
> @@ -63,4 +63,10 @@ struct __old_ipc_perm
> # define __IPC_TIME64 0
> #endif
>
> +#if __IPC_TIME64 || defined __ASSUME_SYSVIPC_BROKEN_MODE_T
> +# define IPC_CTL_NEED_TRANSLATION 1
> +#else
> +# define IPC_CTL_NEED_TRANSLATION 0
> +#endif
> +
> #include <ipc_ops.h>
> diff --git a/sysdeps/unix/sysv/linux/msgctl.c b/sysdeps/unix/sysv/linux/msgctl.c
> index e824ebb095..67b43c0eff 100644
> --- a/sysdeps/unix/sysv/linux/msgctl.c
> +++ b/sysdeps/unix/sysv/linux/msgctl.c
> @@ -85,11 +85,19 @@ msgctl_syscall (int msqid, int cmd, msgctl_arg_t *buf)
> int
> __msgctl64 (int msqid, int cmd, struct __msqid64_ds *buf)
> {
> -#if __IPC_TIME64
> +#if IPC_CTL_NEED_TRANSLATION
> +# if __IPC_TIME64
> struct kernel_msqid64_ds ksemid, *arg = NULL;
> -#else
> +# else
> msgctl_arg_t *arg;
> -#endif
> +# endif
> +
> + /* Some applications pass the __IPC_64 flag in cmd, to invoke
> + previously unsupported commands back when there was no EINVAL
> + error checking in glibc. Mask the flag for the switch statements
> + below. ,sgctl_syscall adds back the __IPC_64 flag for the actual
s/,sgctl_syscall/msgctl_syscall
> + system call. */
> + cmd &= ~__IPC_64;
>
> switch (cmd)
> {
> @@ -101,19 +109,19 @@ __msgctl64 (int msqid, int cmd, struct __msqid64_ds *buf)
> case IPC_STAT:
> case MSG_STAT:
> case MSG_STAT_ANY:
> -#if __IPC_TIME64
> +# if __IPC_TIME64
> if (buf != NULL)
> {
> msqid64_to_kmsqid64 (buf, &ksemid);
> arg = &ksemid;
> }
> -# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> +# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> if (cmd == IPC_SET)
> arg->msg_perm.mode *= 0x10000U;
> -# endif
> -#else
> +# endif
> +# else
> arg = buf;
> -#endif
> +# endif
> break;
>
> case IPC_INFO:
> @@ -137,21 +145,25 @@ __msgctl64 (int msqid, int cmd, struct __msqid64_ds *buf)
> case IPC_STAT:
> case MSG_STAT:
> case MSG_STAT_ANY:
> -#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> +# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> arg->msg_perm.mode >>= 16;
> -#else
> +# else
> /* Old Linux kernel versions might not clear the mode padding. */
> if (sizeof ((struct msqid_ds){0}.msg_perm.mode)
> != sizeof (__kernel_mode_t))
> arg->msg_perm.mode &= 0xFFFF;
> -#endif
> +# endif
>
> -#if __IPC_TIME64
> +# if __IPC_TIME64
> kmsqid64_to_msqid64 (arg, buf);
> -#endif
> +# endif
> }
>
> return ret;
> +
> +#else /* !IPC_CTL_NEED_TRANSLATION */
> + return msgctl_syscall (msqid, cmd, buf);
> +#endif
> }
> #if __TIMESIZE != 64
> libc_hidden_def (__msgctl64)
> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 77a8130c18..3458b018bc 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -140,6 +140,13 @@ __semctl64 (int semid, int semnum, int cmd, ...)
> union semun64 arg64 = { 0 };
> va_list ap;
>
> + /* Some applications pass the __IPC_64 flag in cmd, to invoke
> + previously unsupported commands back when there was no EINVAL
> + error checking in glibc. Mask the flag for the switch statements
> + below. semctl_syscall adds back the __IPC_64 flag for the actual
> + system call. */
> + cmd &= ~__IPC_64;
> +
> /* Get the argument only if required. */
> switch (cmd)
> {
> diff --git a/sysdeps/unix/sysv/linux/shmctl.c b/sysdeps/unix/sysv/linux/shmctl.c
> index ea38935497..f00817a6f6 100644
> --- a/sysdeps/unix/sysv/linux/shmctl.c
> +++ b/sysdeps/unix/sysv/linux/shmctl.c
> @@ -85,11 +85,19 @@ shmctl_syscall (int shmid, int cmd, shmctl_arg_t *buf)
> int
> __shmctl64 (int shmid, int cmd, struct __shmid64_ds *buf)
> {
> -#if __IPC_TIME64
> +#if IPC_CTL_NEED_TRANSLATION
> +# if __IPC_TIME64
> struct kernel_shmid64_ds kshmid, *arg = NULL;
> -#else
> +# else
> shmctl_arg_t *arg;
> -#endif
> +# endif
> +
> + /* Some applications pass the __IPC_64 flag in cmd, to invoke
> + previously unsupported commands back when there was no EINVAL
> + error checking in glibc. Mask the flag for the switch statements
> + below. shmctl_syscall adds back the __IPC_64 flag for the actual
> + system call. */
> + cmd &= ~__IPC_64;
>
> switch (cmd)
> {
> @@ -103,19 +111,19 @@ __shmctl64 (int shmid, int cmd, struct __shmid64_ds *buf)
> case IPC_STAT:
> case SHM_STAT:
> case SHM_STAT_ANY:
> -#if __IPC_TIME64
> +# if __IPC_TIME64
> if (buf != NULL)
> {
> shmid64_to_kshmid64 (buf, &kshmid);
> arg = &kshmid;
> }
> -# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> +# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> if (cmd == IPC_SET)
> arg->shm_perm.mode *= 0x10000U;
> -# endif
> -#else
> +# endif
> +# else
> arg = buf;
> -#endif
> +# endif
> break;
>
> case IPC_INFO:
> @@ -140,21 +148,25 @@ __shmctl64 (int shmid, int cmd, struct __shmid64_ds *buf)
> case IPC_STAT:
> case SHM_STAT:
> case SHM_STAT_ANY:
> -#ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> +# ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> arg->shm_perm.mode >>= 16;
> -#else
> +# else
> /* Old Linux kernel versions might not clear the mode padding. */
> if (sizeof ((struct shmid_ds){0}.shm_perm.mode)
> != sizeof (__kernel_mode_t))
> arg->shm_perm.mode &= 0xFFFF;
> -#endif
> +# endif
>
> -#if __IPC_TIME64
> +# if __IPC_TIME64
> kshmid64_to_shmid64 (arg, buf);
> -#endif
> +# endif
> }
>
> return ret;
> +
> +#else /* !IPC_CTL_NEED_TRANSLATION */
> + return shmctl_syscall (shmid, cmd, buf);
> +#endif
> }
> #if __TIMESIZE != 64
> libc_hidden_def (__shmctl64)
>
> base-commit: 19934d629ee22bbd332f04da4320e4f624c9560c
>
next prev parent reply other threads:[~2022-11-10 12:27 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-08 15:00 Florian Weimer
2022-11-10 10:39 ` Florian Weimer
2022-11-10 12:27 ` Adhemerval Zanella Netto [this message]
2022-11-10 13:28 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ebe416fb-18b9-96ca-bfbf-84aff7a5daeb@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).