linux-kernel - Re: [RFC PATCH 7/9] rv: Retry when da monitor detects race conditions

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250411045225.gP4DqmFt@linutronix.de>
Date: Fri, 11 Apr 2025 06:52:25 +0200
From: Nam Cao <namcao@...utronix.de>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
	linux-trace-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Tomas Glozar <tglozar@...hat.com>, Juri Lelli <jlelli@...hat.com>
Subject: Re: [RFC PATCH 7/9] rv: Retry when da monitor detects race conditions

On Fri, Apr 04, 2025 at 10:45:20AM +0200, Gabriele Monaco wrote:
> DA monitor can be accessed from multiple cores simultaneously, this is
> likely, for instance when dealing with per-task monitors reacting on
> events that do not always occur on the CPU where the task is running.
> This can cause race conditions where two events change the next state
> and we see inconsistent values. E.g.:
> 
>   [62] event_srs: 27: sleepable x sched_wakeup -> running (final)
>   [63] event_srs: 27: sleepable x sched_set_state_sleepable -> sleepable
>   [63] error_srs: 27: event sched_switch_suspend not expected in the state running
> 
> In this case the monitor fails because the event on CPU 62 wins against
> the one on CPU 63, although the correct state should have been
> sleepable, since the task get suspended.
> 
> Detect if the current state was modified by using try_cmpxchg while
> storing the next value. If it was, try again reading the current state.
> After a maximum number of failed retries, react as if it was an error
> with invalid current state (we cannot determine it).
> 
> Monitors where this type of condition can occur must be able to account
> for racing events in any possible order, as we cannot know the winner.

Is locking not simpler? I understand raw_spin_lock() doesn't work because
it steps on some tracepoints. But how about adding something like
raw_spin_lock_notrace()?

static inline bool raw_spin_lock_notrace(raw_spinlock_t *lock)
{
	preempt_disable_notrace(); //probably not required, tracepoint handlers do this already

	if (!do_raw_spin_trylock(lock))
		do_raw_spin_lock(lock);
}

My LTL series theoretically also has this problem, but I have never got it
during testing yet. We should use the same solution for both DA and LTL.

Also, can you please Cc me in your RV patches?

Best regards,
Nam