Re: [PATCH 5.15.y, 6.1.y, 6.6.y] posix-cpu-timers: Prevent UAF caused by non-leader exec()

[Sasha Levin <sashal@kernel.org>]

Thanks for doing these.  I have queued the 6.12 one, but I would like to
ask for a v2 of this one before taking 6.6, 6.1 and 5.15.

> @@ -714,7 +781,7 @@ static int posix_cpu_timer_set(struct k_
>  		 * it as an overrun (thanks to bump_cpu_timer above).
>  		 */
>  		unlock_task_sighand(p, &flags);
> -		goto out;
> +		return ret;
>  	}

[...]

> +	if (old)
> +		old->it_interval = ns_to_timespec64(old_incr);
> +
[...]
> - out:
> -	rcu_read_unlock();
> -	if (old)
> -		old->it_interval = ns_to_timespec64(old_incr);

On these trees that changes user-visible behaviour on the TIMER_RETRY
path.  Before the patch the "goto out" fell through the shared out:
label, which filled in old->it_interval; afterwards the early return
skips it and the assignment only runs on the success path.

Upstream that is harmless because bfa408f03fc7 ("posix-timers: Retrieve
interval in common timer_settime() code") moved the interval retrieval
into the common code, but that landed in v6.12 and 6.6, 6.1 and 5.15 do
not have it.  Their do_timer_settime() sets old_spec64 = NULL before it
goes back around the retry loop, so a timer_settime() that races timer
expiry now reports old_value.it_interval == {0,0} instead of the
previous interval.

Does it make sense?

-- 
Thanks,
Sasha