From: Martin Pieuchot <mpi@openbsd.org>
Subject: Re: pthread_mutex_timedlock(3): don't block forever if the timeout is NULL
To: Scott Cheloha <scottcheloha@gmail.com>
Cc: tech@openbsd.org
Date: Mon, 5 Feb 2024 16:50:18 +0100

On 27/01/24(Sat) 18:19, Scott Cheloha wrote:
> On Tue, Jan 09, 2024 at 11:14:08PM -0600, Scott Cheloha wrote:
> > Both pthread_mutex_timedlock(3) implementations pass the timeout
> > parameter from the caller through to the internal implementation
> > function.  In both of those functions, a NULL timeout pointer means
> > "no timeout" or "block indefinitely".
> > 
> > So, we have accidentally extended the pthread_mutex_timedlock(3)
> > specification with special, undocumented behavior when the timeout
> > pointer is NULL.

Is this a bug?  It sounds sane to me.  Behave just like
pthread_mutex_lock(3).

> > 
> > The easy fix is to copy the timeout to the stack before passing it on.
> > 
> > This change will cause any code that depends upon the current behavior
> > to segfault instead of blocking indefinitely.  I think it's unlikely
> > such code exists.  It it does exist, it is not portable.

I'm not sure adding an explicit NULL-dereference in a library is the way to
go.  If anything it should be better to return an error.  But I see no
evidence that passing a NULL pointer should behave differently than
pthread_mutex_lock(3).

> > 
> > ok?
> 
> Ping.
> 
> Index: rthread_mutex.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/thread/rthread_mutex.c,v
> diff -u -p -r1.5 rthread_mutex.c
> --- rthread_mutex.c	13 Feb 2019 13:09:32 -0000	1.5
> +++ rthread_mutex.c	28 Jan 2024 00:19:09 -0000
> @@ -211,9 +211,11 @@ pthread_mutex_trylock(pthread_mutex_t *m
>  }
>  
>  int
> -pthread_mutex_timedlock(pthread_mutex_t *mutexp, const struct timespec *abs)
> +pthread_mutex_timedlock(pthread_mutex_t *mutexp, const struct timespec *absp)
>  {
> -	return (_rthread_mutex_timedlock(mutexp, 0, abs, 1));
> +	struct timespec abs = *absp;
> +
> +	return (_rthread_mutex_timedlock(mutexp, 0, &abs, 1));
>  }
>  
>  int
> Index: rthread_sync.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/thread/rthread_sync.c,v
> diff -u -p -r1.6 rthread_sync.c
> --- rthread_sync.c	10 Jan 2024 04:28:43 -0000	1.6
> +++ rthread_sync.c	28 Jan 2024 00:19:09 -0000
> @@ -179,9 +179,11 @@ pthread_mutex_trylock(pthread_mutex_t *p
>  }
>  
>  int
> -pthread_mutex_timedlock(pthread_mutex_t *p, const struct timespec *abstime)
> +pthread_mutex_timedlock(pthread_mutex_t *p, const struct timespec *abstimep)
>  {
> -	return (_rthread_mutex_lock(p, 0, abstime));
> +	struct timespec abstime = *abstimep;
> +
> +	return (_rthread_mutex_lock(p, 0, &abstime));
>  }
>  
>  int
>