Re: [PATCH 1/2] kernel/cobalt: Address SIGSHADOW_ACTION_HARDEN and self-hardening race condition

On Sat, 2026-05-16 at 17:48 +0200, Philippe Gerum wrote:
> Jan Kiszka <[email protected]> writes:
> 
> > From: Jan Kiszka <[email protected]>
> > 
> > When a remote thread is suspended, xnthread_suspend checks if it is
> > currently relaxed and then calls it into hardened state via SIGSHADOW.
> > However, this signal may race with the thread entering primary mode on
> > its own, e.g. via a syscall. In that case, the thread will be kicked out
> > of its suspension state by the pending signal, and the migrate syscall
> > it then executes will not perform the expected suspension anymore.
> > 
> > Plug this race by introducing two thread state flags, one for pending
> > XNSUSP and another one for XNHELD. After having performed the requested
> > hardening on SIGSHADOW_ACTION_HARDEN, we are now testing for those
> > pending suspension states and replay them via self-suspension as needed.
> > If a resume took place in the meantime, the state flags were cleared
> > again, and nothing will happen after the migration step.
> > 
> > Reported-by: Richard Weinberger <[email protected]>
> > Signed-off-by: Jan Kiszka <[email protected]>
> > ---
> > 
> > Didn't check yet if EVL solves this topic differently or is exposed to 
> > similar race. But given that Richard found the pattern even in Xenomai 
> > 2, it's likely worth to check EVL carefully.
> > 
> 
> Unlike v3, v4 does not use signals for internal synchronization, so it
> does not suffer that particular issue related to suspending a thread
> (no SIGSHADOW in evl). However, this issue is only a symptom of a much
> broader problem both v3 and v4 have when it comes to dealing with
> signals pending for suspended threads, and your patch won't solve it.
> 
> To illustrate this, let's say (A) and (B) are Cobalt threads running
> oob, (X) may be any in-band context:
> 
> CPU0                                          CPU1
> ----                                          ----
> 
> A: xnthread_signal(B, SIGxx)                  B: core_syscall()
>    <schedule irq_work>                              core_suspend(B, XNPEND)
> A: core_suspend(B, XNSUSP)                    B: <schedules out>
>       core_add_state(B, T_SUSP)
>    core_reschedule()
>       core_resched_ipi(CPU1)                  
> X: irq_work_handler()
>       send_sig(B, SIGxx)
>          handle_inband_event(B, INBAND_TASK_SIGNAL)
>             core_resume(B, XNPEND)
>             core_resume(B, XNSUSP)
>             core_add_state(B, XNKICKED)
>          core_reschedule()
>             core_resched_ipi(CPU1)                    
>                                               X: <ipi-handler>
>                                                     core_reschedule()
>                                               B: <resuming oob>
>                                               B: XNKICKED: prepare_for_signal()
>                                                     xnthread_relax()
>                                               B: <running in-band>
>                                               B: migrate_syscall()
>                                                  nop (XNPNDSUSP was not set)
>                                               B: <still running in-band>
> 
> Remoteness of CPUs does not even matter, e.g. with prio(A) > prio(B),
> we might observe:
> 
> CPU0
> ----
> 
> B: core_syscall()
>       core_suspend(B, XNPEND)
> B: <schedules out>
> X: send_sig(B, SIGxx)
>       handle_inband_event(B, INBAND_TASK_SIGNAL)
>          core_resume(B, XNPEND) /* B is now fully unblocked */
>          core_add_state(B, XNKICKED)
> <IRQ>: core_resume(A)
> A: <resuming oob>
> A: core_suspend(B, XNSUSP)
>       unblocked + XNKICKED set -> abort: XNSUSP ignored
> B: <resuming oob>
> B: XNKICKED: prepare_for_signal()
>       xnthread_relax()
> B: <still running in-band>
> 
> There is clearly no shortage of possible variations of the scenarios
> above especially if we bring up more complex multi-CPU configurations,
> all of them causing some sort of breakage. IOW, there is a general issue
> with signals wrt forcible suspension, not just with SIGSHADOW-based
> synchronization in xnthread_suspend() vs migrate() which in addition,
> has its own set of flaws.
> 
> A way to address this properly may require to break an ancient
> invariant, which states that a thread with any blocking bit set must not
> be linked to any runqueue. On the contrary, a way to make sure that a
> suspend request remains pending across a stage migration in order to
> deal with an in-band signal is precisely to allow that request to linger
> as long as necessary in a thread state mask, only to be considered once
> the 'kicked' flag is cleared, ignored and left pending
> otherwise. Meanwhile, that thread should be allowed to run.
> 
> I need to have a closer look, but at first glance it seems fairly
> straightforward to implement this in v4 at least.
> 
> -- 
> Philippe.

Uh... All of that sounds a bit scary...

There was a reprocuder provided by Richard on the list as well, right?
Please make sure that we integrate a test case at the same time as the
fixes. Otherwise the testcase might never be added.