linux-kernel - Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more complete sts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250730162855.U6IwNmsc@linutronix.de>
Date: Wed, 30 Jul 2025 18:28:55 +0200
From: Nam Cao <namcao@...utronix.de>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
	Jonathan Corbet <corbet@....net>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	linux-trace-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Tomas Glozar <tglozar@...hat.com>, Juri Lelli <jlelli@...hat.com>,
	Clark Williams <williams@...hat.com>,
	John Kacur <jkacur@...hat.com>
Subject: Re: [PATCH v5 7/9] rv: Replace tss and sncid monitors with more
 complete sts

On Mon, Jul 28, 2025 at 05:53:34PM +0200, Nam Cao wrote:
> On Mon, Jul 28, 2025 at 03:50:19PM +0200, Gabriele Monaco wrote:
> > The tss monitor currently guarantees task switches can happen only while
> > scheduling, whereas the sncid monitor enforces scheduling occurs with
> > interrupt disabled.
> > 
> > Replace the monitors with a more comprehensive specification which
> > implies both but also ensures that:
> > * each scheduler call disable interrupts to switch
> > * each task switch happens with interrupts disabled
> > 
> > Cc: Ingo Molnar <mingo@...hat.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Signed-off-by: Gabriele Monaco <gmonaco@...hat.com>
> 
> I gave this a try on riscv64 and observed some errors:
> 
> [  620.696055] rv: monitor sts does not allow event sched_switch on state enable_to_exit
> [  621.047705] rv: monitor sts does not allow event sched_switch on state enable_to_exit
> [  642.440209] rv: monitor sts does not allow event sched_switch on state enable_to_exit
> 
> I tested with two user programs:
> 
>     int main() { asm ("unimp"); }
>     int main() { asm ("ebreak"); }
> 
> The two programs are repeatedly executed:
> 
>     #!/bin/bash
>     ./test1 &
>     ./test2 &
>     # ... repeat lots of time

Okay, I think I know why..

It seems the monitor is in scheduling state. Then it sees a pair of
irq_disable and irq_enable, and it mistakenly thinks that this is the
is_switch==false case in __schedule. So it thinks it is at the end of
__schedule(), and does not expect a switch_switch.

However, this is wrong. The irq_disable and irq_enable pair is not from
__schedule(), it is from softirq (see below).

In short, the monitor thinks it is at the end of __schedule(), but actually
it is still at the beginning.

That's just from my limited understanding of the model, so I may be wrong.
What do you think?

Nam

             test-256     [002] dns..    63.070743: da_event_sts: scheduling x irq_disable -> disable_to_switch
             test-256     [002] dns..    63.070748: <stack trace>
  => trace_dump_stack
  => da_event_sts
  => handle_irq_disable
  => trace_hardirqs_off.part.0
  => trace_hardirqs_off
  => note_gp_changes
  => rcu_core
  => rcu_core_si
  => handle_softirqs
  => __irq_exit_rcu
  => irq_exit_rcu
  => handle_riscv_irq
  => call_on_irq_stack
             test-256     [002] dns..    63.070755: da_event_sts: disable_to_switch x irq_enable -> enable_to_exit
             test-256     [002] dns..    63.070760: <stack trace>
  => trace_dump_stack
  => da_event_sts
  => handle_irq_enable
  => trace_hardirqs_on
  => note_gp_changes
  => rcu_core
  => rcu_core_si
  => handle_softirqs
  => __irq_exit_rcu
  => irq_exit_rcu
  => handle_riscv_irq
  => call_on_irq_stack
  => call_on_irq_stack