unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
@ 2020-10-15 15:17 H.J. Lu via Libc-alpha
  2020-10-15 16:56 ` H.J. Lu via Libc-alpha
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: H.J. Lu via Libc-alpha @ 2020-10-15 15:17 UTC (permalink / raw)
  To: libc-alpha

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;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
  2020-10-15 15:17 [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736] H.J. Lu via Libc-alpha
@ 2020-10-15 16:56 ` H.J. Lu via Libc-alpha
  2020-10-15 17:24 ` Adhemerval Zanella via Libc-alpha
  2020-10-15 17:34 ` Florian Weimer
  2 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu via Libc-alpha @ 2020-10-15 16:56 UTC (permalink / raw)
  To: GNU C Library

On Thu, Oct 15, 2020 at 8:17 AM H.J. Lu <hjl.tools@gmail.com> 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;
> --
> 2.26.2
>

Tested with scripts/build-many-glibcs.py and on i686/x32/x86-64.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
  2020-10-15 15:17 [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736] H.J. Lu via Libc-alpha
  2020-10-15 16:56 ` H.J. Lu via Libc-alpha
@ 2020-10-15 17:24 ` Adhemerval Zanella via Libc-alpha
  2020-10-15 17:40   ` H.J. Lu via Libc-alpha
  2020-10-15 17:34 ` Florian Weimer
  2 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-10-15 17:24 UTC (permalink / raw)
  To: libc-alpha



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;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
  2020-10-15 15:17 [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736] H.J. Lu via Libc-alpha
  2020-10-15 16:56 ` H.J. Lu via Libc-alpha
  2020-10-15 17:24 ` Adhemerval Zanella via Libc-alpha
@ 2020-10-15 17:34 ` Florian Weimer
  2 siblings, 0 replies; 6+ messages in thread
From: Florian Weimer @ 2020-10-15 17:34 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha

* H. J. Lu via Libc-alpha:

> 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

> +    case IPC_INFO:
> +      if (sizeof (((struct shminfo *) 0)->shmmax) > sizeof (uintptr_t))

I think the C way of writing this is:

   sizeof ((struct shminfo) { 0 }.shmmax)

I believe a null pointer deference is still undefined in an
unevaluated sizeof context.

> 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)

Likewise.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
  2020-10-15 17:24 ` Adhemerval Zanella via Libc-alpha
@ 2020-10-15 17:40   ` H.J. Lu via Libc-alpha
  2020-10-15 17:44     ` Adhemerval Zanella via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu via Libc-alpha @ 2020-10-15 17:40 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library

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?

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736]
  2020-10-15 17:40   ` H.J. Lu via Libc-alpha
@ 2020-10-15 17:44     ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 6+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-10-15 17:44 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library



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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-15 17:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 15:17 [PATCH] IPC_INFO: Cast shmmax and shmall fields to uintptr_t [BZ #26736] H.J. Lu via Libc-alpha
2020-10-15 16:56 ` H.J. Lu via Libc-alpha
2020-10-15 17:24 ` Adhemerval Zanella via Libc-alpha
2020-10-15 17:40   ` H.J. Lu via Libc-alpha
2020-10-15 17:44     ` Adhemerval Zanella via Libc-alpha
2020-10-15 17:34 ` Florian Weimer

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).