[PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]

[Adhemerval Zanella]

On 15/10/2020 14:40, H.J. Lu wrote:
> On Thu, Oct 15, 2020 at 10:24 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 15/10/2020 12:17, H.J. Lu via Libc-alpha wrote:
>>> On x32, the shmctl (IPC_INFO) syscall returns the x86-64 values for
>>> shmmax and shmall.  Since x32 is limited to 32-bit address space,
>>> shmmax and shmall should be casted to uintptr_t and shmmax should be
>>> clamped to INT_MAX only if the size of shmmax is the size of int.
>>> ---
>>>  sysdeps/unix/sysv/linux/shmctl.c            | 20 ++++++++++++++++----
>>>  sysdeps/unix/sysv/linux/tst-sysvshm-linux.c |  3 ++-
>>>  2 files changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/shmctl.c b/sysdeps/unix/sysv/linux/shmctl.c
>>> index 833f013e69..8d34c0e7bf 100644
>>> --- a/sysdeps/unix/sysv/linux/shmctl.c
>>> +++ b/sysdeps/unix/sysv/linux/shmctl.c
>>> @@ -138,11 +138,11 @@ __shmctl64 (int shmid, int cmd, struct __shmid64_ds *buf)
>>>
>>>    switch (cmd)
>>>      {
>>> -      case IPC_STAT:
>>> -      case SHM_STAT:
>>> -      case SHM_STAT_ANY:
>>> +    case IPC_STAT:
>>> +    case SHM_STAT:
>>> +    case SHM_STAT_ANY:
>>>  #ifdef __ASSUME_SYSVIPC_BROKEN_MODE_T
>>> -        arg->shm_perm.mode >>= 16;
>>> +      arg->shm_perm.mode >>= 16;
>>>  #else
>>>        /* Old Linux kernel versions might not clear the mode padding.  */
>>>        if (sizeof ((struct shmid_ds){0}.shm_perm.mode)
>>> @@ -153,6 +153,18 @@ __shmctl64 (int shmid, int cmd, struct __shmid64_ds *buf)
>>>  #if __IPC_TIME64
>>>        kshmid64_to_shmid64 (arg, buf);
>>>  #endif
>>> +      break;
>>> +
>>> +    case IPC_INFO:
>>> +      if (sizeof (((struct shminfo *) 0)->shmmax) > sizeof (uintptr_t))
>>> +     {
>>> +       /* Since the address space is limited by the size of uintptr_t,
>>> +          cast shmmax and shmall to uintptr_t.  */
>>> +       struct shminfo *shminfo_p = (struct shminfo *) arg;
>>> +       shminfo_p->shmmax = (uintptr_t) shminfo_p->shmmax;
>>> +       shminfo_p->shmall = (uintptr_t) shminfo_p->shmall;
>>> +     }
>>> +      break;
>>>      }
>>>
>>>    return ret;
>>> diff --git a/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c b/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>>> index 7128ae2e14..abeeb37a78 100644
>>> --- a/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>>> +++ b/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>>> @@ -128,7 +128,8 @@ do_test (void)
>>>  #if LONG_MAX == INT_MAX
>>>      /* Kernel explicit clamp the value for shmmax on compat symbol (32-bit
>>>         binaries running on 64-bit kernels).  */
>>> -    if (v > INT_MAX)
>>> +    if (sizeof (((struct shminfo *) 0)->shmmax) == sizeof (int)
>>> +     && v > INT_MAX)
>>>        v = INT_MAX;
>>>  #endif
>>>      tipcinfo.shmmax = v;
>>>
>>
>> This is semantic change of shmctl for x32, which I really do not oppose
>> but I think we should rely on kernel to clamp the value correctly for x32
>> (the shmctl is already rather complex to handle y2038 safeness).
>>
>> What about changing the testcase to use the expected types for shminfo
>> fields (__syscall_ulong_t instead of unsigned long int) and check for
>> the expected provided kernel value:
>>
>> ---
>>
>> diff --git a/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c b/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>> index 7128ae2e14..cb32bd522e 100644
>> --- a/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>> +++ b/sysdeps/unix/sysv/linux/tst-sysvshm-linux.c
>> @@ -54,9 +54,9 @@ do_prepare (int argc, char *argv[])
>>
>>  struct test_shminfo
>>  {
>> -  unsigned long int shmall;
>> -  unsigned long int shmmax;
>> -  unsigned long int shmmni;
>> +  __syscall_ulong_t shmall;
>> +  __syscall_ulong_t shmmax;
>> +  __syscall_ulong_t shmmni;
>>  };
>>
>>  /* It tries to obtain some system-wide SysV shared memory information from
>> @@ -128,7 +128,8 @@ do_test (void)
>>  #if LONG_MAX == INT_MAX
>>      /* Kernel explicit clamp the value for shmmax on compat symbol (32-bit
>>         binaries running on 64-bit kernels).  */
>> -    if (v > INT_MAX)
>> +    if (sizeof (__syscall_ulong_t) == sizeof (unsigned long int)
>> +        && v > INT_MAX)
>>        v = INT_MAX;
>>  #endif
>>      tipcinfo.shmmax = v;
> 
> This works.  Can you check it in?

I will do it.