]> sourceware.org Git - glibc.git/commitdiff
posix: Fix -Warray-bounds instances building timer_create [BZ #26687]
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>
Mon, 5 Oct 2020 20:30:05 +0000 (17:30 -0300)
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>
Tue, 6 Oct 2020 18:29:35 +0000 (15:29 -0300)
GCC 11 -Warray-bounds triggers invalid warnings when building
Linux timer_create.c:

../sysdeps/unix/sysv/linux/timer_create.c: In function '__timer_create_new':
../sysdeps/unix/sysv/linux/timer_create.c:83:17: warning: array subscript 'struct timer[0]' is partly outside array bounds of 'unsigned char[8]' [-Warray-bounds]
   83 |             newp->sigev_notify = (evp != NULL
      |                 ^~
../sysdeps/unix/sysv/linux/timer_create.c:59:47: note: referencing an object of size 8 allocated by 'malloc'
   59 |         struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                                                                 thrfunc));
      |                                                                 ~~~~~~~~~

The struct allocated for !SIGEV_THREAD timers only requires two 'int'
fields (sigev_notify and ktimerid) and the offsetof trick tries minimize
the memory usage by only allocation the required size.  However,
although the resulting size is suffice for !SIGEV_THREAD time, accessing
the partially allocated object is error-prone and UB.

This patch fixes both issues by embedding the information whether
the timer if a SIGEV_THREAD in the returned 'timer_t'.  For
!SIGEV_THREAD, the resulting 'timer_t' is the returned kernel timer
identifer (kernel_timer_t), while for SIGEV_THREAD it uses the fact
malloc returns at least _Alignof (max_align_t) pointers plus that
valid kernel_timer_t are always positive to set MSB bit of the returned
'timer_t' to indicate the timer handles a SIGEV_THREAD.

It allows to remove the memory allocation for !SIGEV_THREAD and also
remove the 'sigev_notify' field from 'struct timer'.

Checked on x86_64-linux-gnu and i686-linux-gnu.

sysdeps/unix/sysv/linux/kernel-posix-timers.h
sysdeps/unix/sysv/linux/timer_create.c
sysdeps/unix/sysv/linux/timer_delete.c
sysdeps/unix/sysv/linux/timer_getoverr.c
sysdeps/unix/sysv/linux/timer_gettime.c
sysdeps/unix/sysv/linux/timer_settime.c
sysdeps/unix/sysv/linux/x86_64/timer_gettime.c
sysdeps/unix/sysv/linux/x86_64/timer_settime.c

index 4f8d97d3de3e2ebc9e658d9e1782e5ea5f3e9303..6d22b2b0b3f6a1e85b2aa52061011f635c8b2ce8 100644 (file)
@@ -43,21 +43,11 @@ extern pthread_mutex_t __active_timer_sigev_thread_lock attribute_hidden;
 /* Type of timers in the kernel.  */
 typedef int kernel_timer_t;
 
-
-/* Internal representation of timer.  */
+/* Internal representation of SIGEV_THREAD timer.  */
 struct timer
 {
-  /* Notification mechanism.  */
-  int sigev_notify;
-
-  /* Timer ID returned by the kernel.  */
   kernel_timer_t ktimerid;
 
-  /* All new elements must be added after ktimerid.  And if the thrfunc
-     element is not the third element anymore the memory allocation in
-     timer_create needs to be changed.  */
-
-  /* Parameters for the thread to be started for SIGEV_THREAD.  */
   void (*thrfunc) (sigval_t);
   sigval_t sival;
   pthread_attr_t attr;
@@ -65,3 +55,43 @@ struct timer
   /* Next element in list of active SIGEV_THREAD timers.  */
   struct timer *next;
 };
+
+
+/* For !SIGEV_THREAD, the resulting 'timer_t' is the returned kernel timer
+   identifer (kernel_timer_t), while for SIGEV_THREAD it uses the fact malloc
+   returns at least _Alignof (max_align_t) pointers plus that valid
+   kernel_timer_t are always positive to set the MSB bit of the returned
+   'timer_t' to indicate the timer handles a SIGEV_THREAD.  */
+
+static inline timer_t
+kernel_timer_to_timerid (kernel_timer_t ktimerid)
+{
+  return (timer_t) ((intptr_t) ktimerid);
+}
+
+static inline timer_t
+timer_to_timerid (struct timer *ptr)
+{
+  return (timer_t) (INTPTR_MIN | (uintptr_t) ptr >> 1);
+}
+
+static inline bool
+timer_is_sigev_thread (timer_t timerid)
+{
+  return (intptr_t) timerid < 0;
+}
+
+static inline struct timer *
+timerid_to_timer (timer_t timerid)
+{
+  return (struct timer *)((uintptr_t) timerid << 1);
+}
+
+static inline kernel_timer_t
+timerid_to_kernel_timer (timer_t timerid)
+{
+  if (timer_is_sigev_thread (timerid))
+    return timerid_to_timer (timerid)->ktimerid;
+  else
+    return (kernel_timer_t) ((uintptr_t) timerid);
+}
index 370c99a51724f9bdaec5ff9aed650cf32a6ac2c4..18fb00c6e64884fc072c382257302df6ef742917 100644 (file)
@@ -52,16 +52,6 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
       {
        struct sigevent local_evp;
 
-       /* We avoid allocating too much memory by basically
-          using struct timer as a derived class with the
-          first two elements being in the superclass.  We only
-          need these two elements here.  */
-       struct timer *newp = (struct timer *) malloc (offsetof (struct timer,
-                                                               thrfunc));
-       if (newp == NULL)
-         /* No more memory.  */
-         return -1;
-
        if (evp == NULL)
          {
            /* The kernel has to pass up the timer ID which is a
@@ -69,31 +59,17 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
               the kernel to determine it.  */
            local_evp.sigev_notify = SIGEV_SIGNAL;
            local_evp.sigev_signo = SIGALRM;
-           local_evp.sigev_value.sival_ptr = newp;
+           local_evp.sigev_value.sival_ptr = NULL;
 
            evp = &local_evp;
          }
 
        kernel_timer_t ktimerid;
-       int retval = INLINE_SYSCALL (timer_create, 3, syscall_clockid, evp,
-                                    &ktimerid);
-
-       if (retval != -1)
-         {
-           newp->sigev_notify = (evp != NULL
-                                 ? evp->sigev_notify : SIGEV_SIGNAL);
-           newp->ktimerid = ktimerid;
-
-           *timerid = (timer_t) newp;
-         }
-       else
-         {
-           /* Cannot allocate the timer, fail.  */
-           free (newp);
-           retval = -1;
-         }
+       if (INLINE_SYSCALL_CALL (timer_create, syscall_clockid, evp,
+                                &ktimerid) == -1)
+         return -1;
 
-       return retval;
+       *timerid = kernel_timer_to_timerid (ktimerid);
       }
     else
       {
@@ -106,20 +82,18 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
            return -1;
          }
 
-       struct timer *newp;
-       newp = (struct timer *) malloc (sizeof (struct timer));
+       struct timer *newp = malloc (sizeof (struct timer));
        if (newp == NULL)
          return -1;
 
        /* Copy the thread parameters the user provided.  */
        newp->sival = evp->sigev_value;
        newp->thrfunc = evp->sigev_notify_function;
-       newp->sigev_notify = SIGEV_THREAD;
 
        /* We cannot simply copy the thread attributes since the
           implementation might keep internal information for
           each instance.  */
-       (void) pthread_attr_init (&newp->attr);
+       pthread_attr_init (&newp->attr);
        if (evp->sigev_notify_attributes != NULL)
          {
            struct pthread_attr *nattr;
@@ -137,8 +111,7 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
          }
 
        /* In any case set the detach flag.  */
-       (void) pthread_attr_setdetachstate (&newp->attr,
-                                           PTHREAD_CREATE_DETACHED);
+       pthread_attr_setdetachstate (&newp->attr, PTHREAD_CREATE_DETACHED);
 
        /* Create the event structure for the kernel timer.  */
        struct sigevent sev =
@@ -149,27 +122,24 @@ timer_create (clockid_t clock_id, struct sigevent *evp, timer_t *timerid)
 
        /* Create the timer.  */
        int res;
-       res = INTERNAL_SYSCALL_CALL (timer_create,
-                                    syscall_clockid, &sev, &newp->ktimerid);
-       if (INTERNAL_SYSCALL_ERROR_P (res))
+       res = INTERNAL_SYSCALL_CALL (timer_create, syscall_clockid, &sev,
+                                    &newp->ktimerid);
+       if (INTERNAL_SYSCALL_ERROR_P (res))
          {
-           /* Add to the queue of active timers with thread
-              delivery.  */
-           pthread_mutex_lock (&__active_timer_sigev_thread_lock);
-           newp->next = __active_timer_sigev_thread;
-           __active_timer_sigev_thread = newp;
-           pthread_mutex_unlock (&__active_timer_sigev_thread_lock);
-
-           *timerid = (timer_t) newp;
-           return 0;
+           free (newp);
+           __set_errno (INTERNAL_SYSCALL_ERRNO (res));
+           return -1;
          }
 
-       /* Free the resources.  */
-       free (newp);
-
-       __set_errno (INTERNAL_SYSCALL_ERRNO (res));
+       /* Add to the queue of active timers with thread delivery.  */
+       pthread_mutex_lock (&__active_timer_sigev_thread_lock);
+       newp->next = __active_timer_sigev_thread;
+       __active_timer_sigev_thread = newp;
+       pthread_mutex_unlock (&__active_timer_sigev_thread_lock);
 
-       return -1;
+       *timerid = timer_to_timerid (newp);
       }
   }
+
+  return 0;
 }
index c240c06139b3e6663ca75e88c5b3acd1cddb9cc4..c8a5f101b403b780decb887d489a3666c6f88415 100644 (file)
@@ -32,15 +32,15 @@ int
 timer_delete (timer_t timerid)
 {
 #undef timer_delete
-  struct timer *kt = (struct timer *) timerid;
-
-  /* Delete the kernel timer object.  */
-  int res = INLINE_SYSCALL (timer_delete, 1, kt->ktimerid);
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
+  int res = INLINE_SYSCALL_CALL (timer_delete, ktimerid);
 
   if (res == 0)
     {
-      if (kt->sigev_notify == SIGEV_THREAD)
+      if (timer_is_sigev_thread (timerid))
        {
+         struct timer *kt = timerid_to_timer (timerid);
+
          /* Remove the timer from the list.  */
          pthread_mutex_lock (&__active_timer_sigev_thread_lock);
          if (__active_timer_sigev_thread == kt)
@@ -58,10 +58,9 @@ timer_delete (timer_t timerid)
                  prevp = prevp->next;
            }
          pthread_mutex_unlock (&__active_timer_sigev_thread_lock);
-       }
 
-      /* Free the memory.  */
-      (void) free (kt);
+         free (kt);
+       }
 
       return 0;
     }
index 81b9723f0160b7a8ca58dd3385f9a9a3c1d9e078..7862d162b9bb543fa6b086621621cc13499bb725 100644 (file)
@@ -31,10 +31,6 @@ int
 timer_getoverrun (timer_t timerid)
 {
 #undef timer_getoverrun
-  struct timer *kt = (struct timer *) timerid;
-
-  /* Get the information from the kernel.  */
-  int res = INLINE_SYSCALL (timer_getoverrun, 1, kt->ktimerid);
-
-  return res;
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
+  return INLINE_SYSCALL_CALL (timer_getoverrun, ktimerid);
 }
index 5d31c7c864a3915d9e85a54bf754946a435ac1a9..07b6309ba911809e6a52e378cbbb086d7a00be6d 100644 (file)
 int
 __timer_gettime64 (timer_t timerid, struct __itimerspec64 *value)
 {
-  struct timer *kt = (struct timer *) timerid;
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
 
 #ifndef __NR_timer_gettime64
 # define __NR_timer_gettime64 __NR_timer_gettime
 #endif
-  int ret = INLINE_SYSCALL_CALL (timer_gettime64, kt->ktimerid, value);
+  int ret = INLINE_SYSCALL_CALL (timer_gettime64, ktimerid, value);
 #ifndef __ASSUME_TIME64_SYSCALLS
   if (ret == 0 || errno != ENOSYS)
     return ret;
 
   struct itimerspec its32;
-  ret = INLINE_SYSCALL_CALL (timer_gettime, kt->ktimerid, &its32);
+  ret = INLINE_SYSCALL_CALL (timer_gettime, ktimerid, &its32);
   if (ret == 0)
     {
       value->it_interval = valid_timespec_to_timespec64 (its32.it_interval);
index c97a9e92ef018fce3e20a0f2cc3caca5f3752b6a..01c625793855d810e4239bd81d7edd3ac6e522b5 100644 (file)
@@ -28,17 +28,17 @@ __timer_settime64 (timer_t timerid, int flags,
                    const struct __itimerspec64 *value,
                    struct __itimerspec64 *ovalue)
 {
-  struct timer *kt = (struct timer *) timerid;
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
 
 #ifdef __ASSUME_TIME64_SYSCALLS
 # ifndef __NR_timer_settime64
 #  define __NR_timer_settime64 __NR_timer_settime
 # endif
-  return INLINE_SYSCALL_CALL (timer_settime64, kt->ktimerid, flags, value,
+  return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
                               ovalue);
 #else
 # ifdef __NR_timer_settime64
-  int ret = INLINE_SYSCALL_CALL (timer_settime64, kt->ktimerid, flags, value,
+  int ret = INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
                                  ovalue);
   if (ret == 0 || errno != ENOSYS)
     return ret;
@@ -55,7 +55,7 @@ __timer_settime64 (timer_t timerid, int flags,
   its32.it_interval = valid_timespec64_to_timespec (value->it_interval);
   its32.it_value = valid_timespec64_to_timespec (value->it_value);
 
-  int retval = INLINE_SYSCALL_CALL (timer_settime, kt->ktimerid, flags,
+  int retval = INLINE_SYSCALL_CALL (timer_settime, ktimerid, flags,
                                     &its32, ovalue ? &oits32 : NULL);
   if (retval == 0 && ovalue)
     {
index 89002c72433f0858f99fc9c4f9a1c7735434f85a..0e4213d7aab554207ce9dd7f26c05214bf12f05b 100644 (file)
@@ -24,9 +24,9 @@
 int
 __timer_gettime_new (timer_t timerid, struct itimerspec *value)
 {
-  struct timer *kt = (struct timer *) timerid;
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
 
-  return INLINE_SYSCALL_CALL (timer_gettime, kt->ktimerid, value);
+  return INLINE_SYSCALL_CALL (timer_gettime, ktimerid, value);
 }
 versioned_symbol (librt, __timer_gettime_new, timer_gettime, GLIBC_2_3_3);
 
index 7af417459e6eea7526b69c336d4e0208c45d4509..0636f7316d3e5f9972876776c26b5aa17932618b 100644 (file)
@@ -25,10 +25,9 @@ int
 __timer_settime_new (timer_t timerid, int flags, const struct itimerspec *value,
                      struct itimerspec *ovalue)
 {
-  struct timer *kt = (struct timer *) timerid;
+  kernel_timer_t ktimerid = timerid_to_kernel_timer (timerid);
 
-  return INLINE_SYSCALL_CALL (timer_settime, kt->ktimerid, flags, value,
-                              ovalue);
+  return INLINE_SYSCALL_CALL (timer_settime, ktimerid, flags, value, ovalue);
 }
 versioned_symbol (librt, __timer_settime_new, timer_settime, GLIBC_2_3_3);
 
This page took 0.058265 seconds and 5 git commands to generate.