From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32193 invoked by alias); 26 Jun 2019 18:37:45 -0000 Mailing-List: contact glibc-cvs-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: , Sender: glibc-cvs-owner@sourceware.org List-Subscribe: Received: (qmail 32175 invoked by uid 9943); 26 Jun 2019 18:37:45 -0000 Date: Wed, 26 Jun 2019 18:37:00 -0000 Message-ID: <20190626183745.32174.qmail@sourceware.org> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Adhemerval Zanella To: glibc-cvs@sourceware.org Subject: [glibc/azanella/master-posix_clock] nptl: pthread_rwlock: Move timeout validation into _full functions X-Act-Checkin: glibc X-Git-Author: Mike Crowe X-Git-Refname: refs/heads/azanella/master-posix_clock X-Git-Oldrev: f3b67b506f521deda907242ed659371b51014caf X-Git-Newrev: d3c69ac2aa929b766433f2d13bf1cb6a6ab36b7a X-SW-Source: 2019-q2/txt/msg00620.txt.bz2 https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d3c69ac2aa929b766433f2d13bf1cb6a6ab36b7a commit d3c69ac2aa929b766433f2d13bf1cb6a6ab36b7a Author: Mike Crowe Date: Mon Jun 24 12:39:02 2019 +0000 nptl: pthread_rwlock: Move timeout validation into _full functions As recommended by the comments in the implementations of pthread_rwlock_timedrdlock and pthread_rwlock_timedwrlock, let's move the timeout validity checks into the corresponding pthread_rwlock_rdlock_full and pthread_rwlock_wrlock_full functions. Since these functions may be called with abstime == NULL, an extra check for that is necessary too. * nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full): Check validity of abstime parameter. (__pthread_rwlock_rwlock_full): Likewise. * nptl/pthread_rwlock_timedrdlock.c * (pthread_rwlock_timedrdlock): Remove check for validity of abstime parameter. * nptl/pthread_rwlock_timedwrlock.c * (pthread_rwlock_timedwrlock): Likewise. Reviewed-by: Adhemerval Zanella Diff: --- ChangeLog | 8 ++++++++ nptl/pthread_rwlock_common.c | 20 ++++++++++++++++++++ nptl/pthread_rwlock_timedrdlock.c | 10 ---------- nptl/pthread_rwlock_timedwrlock.c | 10 ---------- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index 10589c7..3f28206 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2019-06-26 Mike Crowe + * nptl/pthread_rwlock_common.c (__pthread_rwlock_rdlock_full): + Check validity of abstime parameter. + (__pthread_rwlock_rwlock_full): Likewise. + * nptl/pthread_rwlock_timedrdlock.c (pthread_rwlock_timedrdlock): + Remove check for validity of abstime parameter. + * nptl/pthread_rwlock_timedwrlock.c (pthread_rwlock_timedwrlock): + Likewise. + * nptl/Makefile: Add tst-cond26 and tst-cond27 * nptl/Versions (GLIBC_2.30): Add pthread_cond_clockwait * sysdeps/nptl/pthread.h: Likewise diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c index 89ba21a..120b880 100644 --- a/nptl/pthread_rwlock_common.c +++ b/nptl/pthread_rwlock_common.c @@ -282,6 +282,16 @@ __pthread_rwlock_rdlock_full (pthread_rwlock_t *rwlock, { unsigned int r; + /* Make sure any passed in timeout value is valid. Note that the previous + implementation assumed that this check *must* not be performed if there + would in fact be no blocking; however, POSIX only requires that "the + validity of the abstime parameter need not be checked if the lock can be + immediately acquired" (i.e., we need not but may check it). */ + if (abstime + && __glibc_unlikely (abstime->tv_nsec >= 1000000000 + || abstime->tv_nsec < 0)) + return EINVAL; + /* Make sure we are not holding the rwlock as a writer. This is a deadlock situation we recognize and report. */ if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer) @@ -576,6 +586,16 @@ static __always_inline int __pthread_rwlock_wrlock_full (pthread_rwlock_t *rwlock, const struct timespec *abstime) { + /* Make sure any passed in timeout value is valid. Note that the previous + implementation assumed that this check *must* not be performed if there + would in fact be no blocking; however, POSIX only requires that "the + validity of the abstime parameter need not be checked if the lock can be + immediately acquired" (i.e., we need not but may check it). */ + if (abstime + && __glibc_unlikely (abstime->tv_nsec >= 1000000000 + || abstime->tv_nsec < 0)) + return EINVAL; + /* Make sure we are not holding the rwlock as a writer. This is a deadlock situation we recognize and report. */ if (__glibc_unlikely (atomic_load_relaxed (&rwlock->__data.__cur_writer) diff --git a/nptl/pthread_rwlock_timedrdlock.c b/nptl/pthread_rwlock_timedrdlock.c index aa00530..84c1983 100644 --- a/nptl/pthread_rwlock_timedrdlock.c +++ b/nptl/pthread_rwlock_timedrdlock.c @@ -23,15 +23,5 @@ int pthread_rwlock_timedrdlock (pthread_rwlock_t *rwlock, const struct timespec *abstime) { - /* Make sure the passed in timeout value is valid. Note that the previous - implementation assumed that this check *must* not be performed if there - would in fact be no blocking; however, POSIX only requires that "the - validity of the abstime parameter need not be checked if the lock can be - immediately acquired" (i.e., we need not but may check it). */ - /* ??? Just move this to __pthread_rwlock_rdlock_full? */ - if (__glibc_unlikely (abstime->tv_nsec >= 1000000000 - || abstime->tv_nsec < 0)) - return EINVAL; - return __pthread_rwlock_rdlock_full (rwlock, abstime); } diff --git a/nptl/pthread_rwlock_timedwrlock.c b/nptl/pthread_rwlock_timedwrlock.c index 3c92e44..f0b745d 100644 --- a/nptl/pthread_rwlock_timedwrlock.c +++ b/nptl/pthread_rwlock_timedwrlock.c @@ -23,15 +23,5 @@ int pthread_rwlock_timedwrlock (pthread_rwlock_t *rwlock, const struct timespec *abstime) { - /* Make sure the passed in timeout value is valid. Note that the previous - implementation assumed that this check *must* not be performed if there - would in fact be no blocking; however, POSIX only requires that "the - validity of the abstime parameter need not be checked if the lock can be - immediately acquired" (i.e., we need not but may check it). */ - /* ??? Just move this to __pthread_rwlock_wrlock_full? */ - if (__glibc_unlikely (abstime->tv_nsec >= 1000000000 - || abstime->tv_nsec < 0)) - return EINVAL; - return __pthread_rwlock_wrlock_full (rwlock, abstime); }