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

[Robert Elz <kre%munnari.OZ.AU@localhost>]

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.