Re: kern/58733: clock_nanosleep() (and I suspect more) is badly broken

["Robert Elz via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/58733; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/58733: clock_nanosleep() (and I suspect more) is badly broken
Date: Wed, 09 Oct 2024 15:28:32 +0700

 This is the simple change I am planning (sans objections giving a
 reason why this is the wrong way) to make to fix (I believe, I haven't
 yet tested it) this issue.
 
 Index: subr_time.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/subr_time.c,v
 retrieving revision 1.38
 diff -u -r1.38 subr_time.c
 --- subr_time.c	8 Jul 2023 20:02:10 -0000	1.38
 +++ subr_time.c	9 Oct 2024 08:15:44 -0000
 @@ -331,7 +331,8 @@
  	if ((flags & TIMER_ABSTIME) != 0) {
  		if (!timespecsubok(ts, &tsd))
  			return EINVAL;
 -		timespecsub(ts, &tsd, ts);
 +		timespecsub(ts, &tsd, &tsd);
 +		ts = &tsd;
  	}
  
  	error = itimespecfix(ts);
 
 
 
 I had to (for this simple change) omit the (desirable) addition
 of the "const" to the parameter, as that would apply to "ts" and
 while it is no issue for timespecsubok() (and certainly not for
 timespecsub() which is just a macro, and where the change would
 actually benefit, as the compiler could check bad things don't
 happen) it isn't OK for itimespecfix() which wants to modify its
 arg, so can't be passed a const * pointer.    A little more code,
 and an extra variable, would fix that, but that's just a frill
 really, the real objective is to have the bug fixed.
 
 Further, it is possible that callers using TIMER_RELTIME instead
 of TIMER_ABSTIME are actually expecting the change that is made,
 when needed, by itimespecfix() to be reflected back to the passed
 in timespec - that I haven't audited (and understanding what lwp_park()
 is doing in that case is more than I care to do, really...) - I
 didn't even bother to look at the one or two cases of ts2timo() which
 always use TIMER_RELTIME as they cannot be affected by the issue of
 concern - as seen above, the change is confined to the TIMER_ABSTIME
 case.
 
 I will also make the change suggested in the PR to ts2timo(9) if
 we proceed with this method of fetching things.