diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2020-10-05 17:30:05 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2020-10-06 15:29:35 -0300 |
commit | 7a887dd537cd00fe3cdf42b788b3f0e3b430b0ed (patch) | |
tree | 2ce82095bd3f24a54af4980703cbb69cd5b7d155 | |
parent | 862897d2addfacc2af85b571ebf5a82659455e8c (diff) | |
download | glibc-7a887dd537cd00fe3cdf42b788b3f0e3b430b0ed.tar.gz glibc-7a887dd537cd00fe3cdf42b788b3f0e3b430b0ed.tar.xz glibc-7a887dd537cd00fe3cdf42b788b3f0e3b430b0ed.zip |
posix: Fix -Warray-bounds instances building timer_create [BZ #26687]
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.
-rw-r--r-- | sysdeps/unix/sysv/linux/kernel-posix-timers.h | 52 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/timer_create.c | 74 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/timer_delete.c | 15 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/timer_getoverr.c | 8 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/timer_gettime.c | 6 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/timer_settime.c | 8 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/x86_64/timer_gettime.c | 4 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/x86_64/timer_settime.c | 5 |
8 files changed, 83 insertions, 89 deletions
diff --git a/sysdeps/unix/sysv/linux/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h index 4f8d97d3de..6d22b2b0b3 100644 --- a/sysdeps/unix/sysv/linux/kernel-posix-timers.h +++ b/sysdeps/unix/sysv/linux/kernel-posix-timers.h @@ -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); +} diff --git a/sysdeps/unix/sysv/linux/timer_create.c b/sysdeps/unix/sysv/linux/timer_create.c index 370c99a517..18fb00c6e6 100644 --- a/sysdeps/unix/sysv/linux/timer_create.c +++ b/sysdeps/unix/sysv/linux/timer_create.c @@ -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; } diff --git a/sysdeps/unix/sysv/linux/timer_delete.c b/sysdeps/unix/sysv/linux/timer_delete.c index c240c06139..c8a5f101b4 100644 --- a/sysdeps/unix/sysv/linux/timer_delete.c +++ b/sysdeps/unix/sysv/linux/timer_delete.c @@ -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; } diff --git a/sysdeps/unix/sysv/linux/timer_getoverr.c b/sysdeps/unix/sysv/linux/timer_getoverr.c index 81b9723f01..7862d162b9 100644 --- a/sysdeps/unix/sysv/linux/timer_getoverr.c +++ b/sysdeps/unix/sysv/linux/timer_getoverr.c @@ -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); } diff --git a/sysdeps/unix/sysv/linux/timer_gettime.c b/sysdeps/unix/sysv/linux/timer_gettime.c index 5d31c7c864..07b6309ba9 100644 --- a/sysdeps/unix/sysv/linux/timer_gettime.c +++ b/sysdeps/unix/sysv/linux/timer_gettime.c @@ -26,18 +26,18 @@ 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); diff --git a/sysdeps/unix/sysv/linux/timer_settime.c b/sysdeps/unix/sysv/linux/timer_settime.c index c97a9e92ef..01c6257938 100644 --- a/sysdeps/unix/sysv/linux/timer_settime.c +++ b/sysdeps/unix/sysv/linux/timer_settime.c @@ -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) { diff --git a/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c b/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c index 89002c7243..0e4213d7aa 100644 --- a/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c +++ b/sysdeps/unix/sysv/linux/x86_64/timer_gettime.c @@ -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); diff --git a/sysdeps/unix/sysv/linux/x86_64/timer_settime.c b/sysdeps/unix/sysv/linux/x86_64/timer_settime.c index 7af417459e..0636f7316d 100644 --- a/sysdeps/unix/sysv/linux/x86_64/timer_settime.c +++ b/sysdeps/unix/sysv/linux/x86_64/timer_settime.c @@ -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); |