Index | Thread | Search

From:
Scott Cheloha <scottcheloha@gmail.com>
Subject:
Re: pthread_cond_timedwait(3): negative absolute timeouts are not EINVAL
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
tech@openbsd.org
Date:
Mon, 8 Jan 2024 12:57:28 -0600

Download raw body.

Thread
On Mon, Jan 08, 2024 at 11:06:45AM +0100, Mark Kettenis wrote:
> > Date: Sun, 7 Jan 2024 18:59:09 -0600
> > From: Scott Cheloha <scottcheloha@gmail.com>
> > 
> > All values of tv_sec are valid for absolute timeouts.
> > 
> > ok?
> 
> I think that if we allow "negative" absolute times, the code that
> checks if the time has passed in _twait() may fail due to integer
> wraps?

Yes, but I committed a fix for that problem yesterday:

http://cvsweb.openbsd.org/src/lib/libc/thread/synch.h?rev=1.9&content-type=text/x-cvsweb-markup

> libc, librthread: _twait: subtraction is not comparison
> 
> Compare the current time with the absolute timeout before computing
> the relative timeout to avoid arithmetic overflow.  Fixes a bug where
> large negative absolute timeouts are subtracted into large positive
> relative timeouts and incorrectly cause the caller to block.
> 
> While here, use timespeccmp(3) and timespecsub(3) to simplify the
> code.
> 
> Thread: https://marc.info/?l=openbsd-tech&m=169945962503129&w=2

> > Index: rthread_cond.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libc/thread/rthread_cond.c,v
> > diff -u -p -r1.5 rthread_cond.c
> > --- rthread_cond.c	29 Jan 2019 17:40:26 -0000	1.5
> > +++ rthread_cond.c	8 Jan 2024 00:55:09 -0000
> > @@ -142,8 +142,7 @@ pthread_cond_timedwait(pthread_cond_t *c
> >  	}
> >  
> >  	cond = *condp;
> > -	if (abs == NULL || abs->tv_sec < 0 || abs->tv_nsec < 0 ||
> > -	    abs->tv_nsec >= 1000000000)
> > +	if (abs == NULL || abs->tv_nsec < 0 || abs->tv_nsec >= 1000000000)
> >  		return (EINVAL);
> >  
> >  	return (_rthread_cond_timedwait(cond, mutexp, abs));
> > Index: rthread_sync.c
> > ===================================================================
> > RCS file: /cvs/src/lib/libc/thread/rthread_sync.c,v
> > diff -u -p -r1.5 rthread_sync.c
> > --- rthread_sync.c	24 Apr 2018 16:28:42 -0000	1.5
> > +++ rthread_sync.c	8 Jan 2024 00:55:09 -0000
> > @@ -317,7 +317,7 @@ pthread_cond_timedwait(pthread_cond_t *c
> >  			abort();
> >  	}
> >  
> > -	if (abstime == NULL || abstime->tv_sec < 0 || abstime->tv_nsec < 0 ||
> > +	if (abstime == NULL || abstime->tv_nsec < 0 ||
> >  	    abstime->tv_nsec >= 1000000000)
> >  		return (EINVAL);
> >  
> > 
> >