Re: [patch V2 26/50] signal: Get rid of resched_timer logic

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, Apr 19 2024 at 13:06, Oleg Nesterov wrote:
> On 04/18, Oleg Nesterov wrote:
>> But this is not simple, collect_signal() checks SIGQUEUE_PREALLOC exactly
>> because (iiuc) we need to ensure that SI_TIMER didn't come from userspace.
>>
>> perhaps we should disallow SI_TIMER with _sys_private != 0 from userspace,
>> I dunno...
>>
>> And I don't really understand the "not to be passed to user" comment in
>> include/uapi/asm-generic/siginfo.h. copy_siginfo_to_user() just copies
>> the whole kernel_siginfo.
>
> OK, si_sys_private is cleared in dequeue_signal() (or in posixtimer_rearm()
> with this series).
>
> In the past SI_TIMER was defined as __SI_CODE(__SI_TIMER,-2), it was > 0,
> so it could not come from userspace (see the info->si_code >= 0 check in
> do_rt_sigqueueinfo).

Duh.

> Today SI_TIMER < 0. We could introduce SI_TIMER_KERNEL > 0 with the minimal
> changes, but this can't help because the commit 66dd34ad31e59 allows to send
> any siginfo to itself.

Well that predates the __SI_CODE() removal. So I doubt it's required
today, but what do I know about CRIU.

> Otoh, I have no idea how CRIU restores the posix timers. If a process has
> a pending blocked SI_TIMER signal, then I guess it actually needs to enqueue
> this signal at restore time, but resched_timer will be never true?

It can't restore the correct sys_si_private value because that is
nowhere exposed to user space.

There is no special treatment for SI_TIMER, so the signal restore might
just end up queueing a random extra SI_TIMER signal if there was one
pending.

I checked the CRIU source and it looks like this just "works" by
reconstructing and rearming the timer with the last expiry value. As
that is in the past it will fire immediately and queue the signal.

sys_si_private seems to be excluded from being set from user space in
compat mode, but in non-compat mode I can't find anything which prevents
that.

Let me stare more at this. 

Thanks,

        tglx