[PATCH v7 2/2] sysv: linux: Pass 64-bit version of semctl syscall

Alistair Francis alistair23@gmail.com
Tue May 5 18:22:05 GMT 2020


On Tue, May 5, 2020 at 10:29 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Alistair,
>
> > The semctl_syscall() function passes a union semun to the kernel. The
> > union includes struct semid_ds as a member. On 32-bit architectures
> > the Linux kernel provides a *_high version of the 32-bit sem_otime and
> > sem_ctime values. These can be combined to get a 64-bit version of the
> > time.
> >
> > This patch adjusts the struct semid_ds to support the *_high versions
> > of sem_otime and sem_ctime. For 32-bit systems with a 64-bit time_t
> > this can be used to get a 64-bit time from the two 32-bit values.
> >
> > This change handles issues that would arrise with architectures that
> > might select __TIMESIZE at build time (and result in different size
> > semid_ds structs).
> >
> > As described by Adhemerval:
> >   1. If an ABI issues 'semctl (semid, semnum, IPC_STAT, ...)' with
> >      IPC_STAT without _IPC_CMD_TIME_64, the provided semid_ds buffer
> >      will be used directly on the syscall.  This is the case for
> >      64-bit architectures and 32-bit architecture without
> >      __TIMESIZE==32 support (RV32 for instance).
> >
> >   2. If an ABI issues 'semctl (semid, semnum, IPC_STAT, ...)' with
> >      IPC_STAT with _IPC_CMD_TIME_64 then __TIMESIZE==64 is implied.
> >      A temporary __semid_ds32 will be used to issue the syscall and
> > its contents will be copied to users semid_ds buffer (which is also
> >      implied to be the __TIMESIZE==64).
>
> Shall I provide any extra configuration (i.e. any extra flags) to have
> support for 64 bit time for this syscall on machines with __WORDSIZE ==
> 32 && __TIMESIZE != 64 (i.e. ARM 32 bit)?

I don't think so. This should "just work" with __TIMESIZE == 64 for ARM.

I have a change for RV32 (ipctypes.h) to support RV32 as __TIMESIZE is
always 64.

>
> Please correct me if I'm wrong, but from the below code (at
> bits/ipctypes.h) it seems like #define __IPC_CMD_TIME64 0x0 is for the
> above case (ARM 32 bit).

It should be 0 by default and 0x100 for __TIMESIZE == 64.

Alistair

>
> > ---
> >  bits/ipctypes.h                             |  10 ++
> >  sysdeps/mips/bits/ipctypes.h                |  10 ++
> >  sysdeps/unix/sysv/linux/bits/ipc.h          |   3 +-
> >  sysdeps/unix/sysv/linux/bits/sem.h          |   4 +-
> >  sysdeps/unix/sysv/linux/ipc_priv.h          |   4 +
> >  sysdeps/unix/sysv/linux/semctl.c            | 121
> > ++++++++++++++++---- sysdeps/unix/sysv/linux/x86/bits/ipctypes.h |
> > 13 +++ 7 files changed, 141 insertions(+), 24 deletions(-)
> >
> > diff --git a/bits/ipctypes.h b/bits/ipctypes.h
> > index a955cdda7f..1b94122c32 100644
> > --- a/bits/ipctypes.h
> > +++ b/bits/ipctypes.h
> > @@ -32,5 +32,15 @@ typedef unsigned short int __ipc_pid_t;
> >  typedef int __ipc_pid_t;
> >  # endif
> >
> > +#if __WORDSIZE == 64                                           \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > +# define __IPC_CMD_TIME64 0x0
> > +#else
> > +# if __TIMESIZE == 64
> > +#  define __IPC_CMD_TIME64 0x100  /* Same as __IPC_64.  */
> > +# else
> > +#  define __IPC_CMD_TIME64 0x0
> > +# endif
> > +#endif
> >
> >  #endif /* bits/ipctypes.h */
> > diff --git a/sysdeps/mips/bits/ipctypes.h
> > b/sysdeps/mips/bits/ipctypes.h index 518f579b36..9a7218bf7d 100644
> > --- a/sysdeps/mips/bits/ipctypes.h
> > +++ b/sysdeps/mips/bits/ipctypes.h
> > @@ -27,5 +27,15 @@
> >
> >  typedef __SLONG32_TYPE __ipc_pid_t;
> >
> > +#if __WORDSIZE == 64                                           \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > +# define __IPC_CMD_TIME64 0x0
> > +#else
> > +# if __TIMESIZE == 64
> > +#  define __IPC_CMD_TIME64 0x100  /* Same as __IPC_64.  */
> > +# else
> > +#  define __IPC_CMD_TIME64 0x0
> > +# endif
> > +#endif
> >
> >  #endif /* bits/ipctypes.h */
> > diff --git a/sysdeps/unix/sysv/linux/bits/ipc.h
> > b/sysdeps/unix/sysv/linux/bits/ipc.h index 085dd628ac..2519322d16
> > 100644 --- a/sysdeps/unix/sysv/linux/bits/ipc.h
> > +++ b/sysdeps/unix/sysv/linux/bits/ipc.h
> > @@ -20,6 +20,7 @@
> >  #endif
> >
> >  #include <bits/types.h>
> > +#include <bits/ipctypes.h>
> >
> >  /* Mode bits for `msgget', `semget', and `shmget'.  */
> >  #define IPC_CREAT    01000           /* Create key if key
> > does not exist. */ @@ -29,7 +30,7 @@
> >  /* Control commands for `msgctl', `semctl', and `shmctl'.  */
> >  #define IPC_RMID     0               /* Remove identifier.  */
> >  #define IPC_SET              1               /* Set `ipc_perm'
> > options.  */ -#define IPC_STAT        2               /* Get
> > `ipc_perm' options.  */ +#define IPC_STAT     (2 |
> > __IPC_CMD_TIME64)             /* Get `ipc_perm' options.  */
> > #ifdef __USE_GNU # define IPC_INFO    3               /* See
> > ipcs.  */ #endif
> > diff --git a/sysdeps/unix/sysv/linux/bits/sem.h
> > b/sysdeps/unix/sysv/linux/bits/sem.h index ba1169fdb3..8d0b88f5e0
> > 100644 --- a/sysdeps/unix/sysv/linux/bits/sem.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sem.h
> > @@ -54,9 +54,9 @@
> >  #ifdef __USE_MISC
> >
> >  /* ipcs ctl cmds */
> > -# define SEM_STAT 18
> > +# define SEM_STAT (18 | __IPC_CMD_TIME64)
> >  # define SEM_INFO 19
> > -# define SEM_STAT_ANY 20
> > +# define SEM_STAT_ANY (20 | __IPC_CMD_TIME64)
> >
> >  struct  seminfo
> >  {
> > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h
> > b/sysdeps/unix/sysv/linux/ipc_priv.h index 15a6e683a4..a1a7cacd17
> > 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h
> > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h
> > @@ -43,6 +43,10 @@ struct __old_ipc_perm
> >    unsigned short int __seq;          /* Sequence number.  */
> >  };
> >
> > +#define __IPC_TIME64 \
> > + (__WORDSIZE == 32 && __TIMESIZE == 64 \
> > +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> > +
> >  #define SEMCTL_ARG_ADDRESS(__arg) &__arg.array
> >
> >  #define MSGRCV_ARGS(__msgp, __msgtyp) \
> > diff --git a/sysdeps/unix/sysv/linux/semctl.c
> > b/sysdeps/unix/sysv/linux/semctl.c index 30571af49f..99ad686194 100644
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -22,12 +22,16 @@
> >  #include <sysdep.h>
> >  #include <shlib-compat.h>
> >  #include <errno.h>
> > +#include <struct__semid_ds32.h>
> >  #include <linux/posix_types.h>  /* For __kernel_mode_t.  */
> >
> >  /* Define a `union semun' suitable for Linux here.  */
> >  union semun
> >  {
> >    int val;                   /* value for SETVAL */
> > +#if __IPC_TIME64
> > +  struct __semid_ds32 *buf32;
> > +#endif
> >    struct semid_ds *buf;              /* buffer for IPC_STAT &
> > IPC_SET */ unsigned short int *array; /* array for GETALL &
> > SETALL */ struct seminfo *__buf;      /* buffer for IPC_INFO */
> > @@ -44,13 +48,54 @@ union semun
> >  static int
> >  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> >  {
> > +  int ret;
> > +#if __IPC_TIME64
> > +  /* A temporary buffer is used to avoid both an issue where the
> > export
> > +     semid_ds might not follow the kernel's expected layout (due
> > +     to {o,c}time{_high} alignment in 64-bit time case) and the
> > issue where
> > +     some kernel versions might not clear the high bits when
> > returning
> > +     then {o,c}time{_high} information.  */
> > +  struct __semid_ds32 tmp;
> > +  struct semid_ds *orig;
> > +  bool restore = false;
> > +  if (cmd == IPC_STAT || cmd == SEM_STAT || cmd == SEM_STAT_ANY)
> > +    {
> > +      tmp = (struct __semid_ds32) {
> > +        .sem_perm  = arg.buf->sem_perm,
> > +        .sem_otime = arg.buf->sem_otime,
> > +        .sem_otime_high = arg.buf->sem_otime >> 32,
> > +        .sem_ctime = arg.buf->sem_ctime,
> > +        .sem_ctime_high = arg.buf->sem_ctime >> 32,
> > +        .sem_nsems = arg.buf->sem_nsems,
> > +      };
> > +      orig = arg.buf;
> > +      arg.buf = (struct semid_ds*) &tmp;
> > +      restore = true;
> > +    }
> > +#endif
> > +
> >  #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> > -  return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > -                           arg.array);
> > +  ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > +                             arg.array);
> >  #else
> > -  return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd
> > | __IPC_64,
> > -                           SEMCTL_ARG_ADDRESS (arg));
> > +  ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd |
> > __IPC_64,
> > +                             SEMCTL_ARG_ADDRESS (arg));
> >  #endif
> > +
> > +#if __IPC_TIME64
> > +  if (ret >= 0 && restore)
> > +    {
> > +      arg.buf = orig;
> > +      arg.buf->sem_perm  = tmp.sem_perm;
> > +      arg.buf->sem_otime = tmp.sem_otime
> > +                           | ((__time64_t) tmp.sem_otime_high << 32);
> > +      arg.buf->sem_ctime = tmp.sem_ctime
> > +                           | ((__time64_t) tmp.sem_ctime_high << 32);
> > +      arg.buf->sem_nsems = tmp.sem_nsems;
> > +    }
> > +#endif
> > +
> > +    return ret;
> >  }
> >
> >  int
> > @@ -64,15 +109,15 @@ __new_semctl (int semid, int semnum, int cmd,
> > ...) union semun arg = { 0 };
> >    va_list ap;
> >
> > -  /* Get the argument only if required.  */
> > -  switch (cmd)
> > +  /* Get the argument only if required.  Ignore the
> > __IPC_CMD_TIME64.  */
> > +  switch (cmd & ~__IPC_CMD_TIME64)
> >      {
> >      case SETVAL:        /* arg.val */
> >      case GETALL:        /* arg.array */
> >      case SETALL:
> > -    case IPC_STAT:      /* arg.buf */
> > +    case IPC_STAT & ~__IPC_CMD_TIME64: /* arg.buf */
> >      case IPC_SET:
> > -    case SEM_STAT:
> > +    case SEM_STAT & ~__IPC_CMD_TIME64:
> >      case IPC_INFO:      /* arg.__buf */
> >      case SEM_INFO:
> >        va_start (ap, cmd);
> > @@ -81,6 +126,20 @@ __new_semctl (int semid, int semnum, int cmd, ...)
> >        break;
> >      }
> >
> > +#ifdef __IPC_TIME64
> > +  struct __semid_ds32 tmp32;
> > +  struct semid_ds *orig64;
> > +  bool restore = false;
> > +
> > +  if (cmd & __IPC_CMD_TIME64)
> > +    {
> > +       tmp32 = (struct __semid_ds32) { 0 };
> > +       orig64 = (struct semid_ds *) arg.buf;
> > +       arg.buf = (struct semid_ds *) &tmp32;
> > +       restore = true;
> > +    }
> > +#endif
> > +
> >  #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> >    struct semid_ds tmpds;
> >    if (cmd == IPC_SET)
> > @@ -91,26 +150,46 @@ __new_semctl (int semid, int semnum, int cmd,
> > ...) }
> >  #endif
> >
> > -  int ret = semctl_syscall (semid, semnum, cmd, arg);
> > +  int ret = semctl_syscall (semid, semnum, cmd & ~__IPC_CMD_TIME64,
> > arg);
> > +  if (ret < 0)
> > +    return ret;
> >
> > -  if (ret >= 0)
> > +  switch (cmd & ~__IPC_CMD_TIME64)
> >      {
> > -      switch (cmd)
> > -     {
> > -        case IPC_STAT:
> > -        case SEM_STAT:
> > -        case SEM_STAT_ANY:
> > +    case IPC_STAT:
> > +    case SEM_STAT:
> > +    case SEM_STAT_ANY:
> >  #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
> > -          arg.buf->sem_perm.mode >>= 16;
> > +# if __IPC_TIME64
> > +      arg.buf32->sem_perm.mode >>= 16;
> > +# else
> > +      arg.buf->sem_perm.mode >>= 16;
> > +# endif
> >  #else
> > -       /* Old Linux kernel versions might not clear the mode
> > padding.  */
> > -       if (sizeof ((struct semid_ds){0}.sem_perm.mode)
> > -           != sizeof (__kernel_mode_t))
> > -         arg.buf->sem_perm.mode &= 0xFFFF;
> > +      /* Old Linux kernel versions might not clear the mode padding.
> >  */
> > +      if (sizeof ((struct semid_ds){0}.sem_perm.mode)
> > +          != sizeof (__kernel_mode_t))
> > +# if __IPC_TIME64
> > +        arg.buf32->sem_perm.mode &= 0xFFFF;
> > +# else
> > +        arg.buf->sem_perm.mode &= 0xFFFF;
> > +# endif
> >  #endif
> > -     }
> >      }
> >
> > +#ifdef __IPC_TIME64
> > +  if (restore)
> > +    {
> > +      orig64->sem_perm = tmp32.sem_perm;
> > +      orig64->sem_otime = tmp32.sem_otime
> > +                          | ((__time64_t) tmp32.sem_otime_high <<
> > 32);
> > +      orig64->sem_ctime = tmp32.sem_ctime
> > +                          | ((__time64_t) tmp32.sem_ctime_high <<
> > 32);
> > +      orig64->sem_nsems = tmp32.sem_nsems;
> > +      arg.buf = (struct semid_ds *) orig64;
> > +    }
> > +#endif
> > +
> >    return ret;
> >  }
> >  versioned_symbol (libc, __new_semctl, semctl, DEFAULT_VERSION);
> > diff --git a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h
> > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h index
> > 12e4cd7682..aa56da6cd5 100644 ---
> > a/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h +++
> > b/sysdeps/unix/sysv/linux/x86/bits/ipctypes.h @@ -23,6 +23,8 @@
> >  #ifndef _BITS_IPCTYPES_H
> >  #define _BITS_IPCTYPES_H     1
> >
> > +#include <bits/types.h>
> > +
> >  /* Used in `struct shmid_ds'.  */
> >  # ifdef __x86_64__
> >  typedef int __ipc_pid_t;
> > @@ -30,4 +32,15 @@ typedef int __ipc_pid_t;
> >  typedef unsigned short int __ipc_pid_t;
> >  # endif
> >
> > +#if __WORDSIZE == 64                                           \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
> > +# define __IPC_CMD_TIME64 0x0
> > +#else
> > +# if __TIMESIZE == 64
> > +#  define __IPC_CMD_TIME64 0x100  /* Same as __IPC_64.  */
> > +# else
> > +#  define __IPC_CMD_TIME64 0x0
> > +# endif
> > +#endif
> > +
> >  #endif /* bits/ipctypes.h */
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de


More information about the Libc-alpha mailing list