linux-kernel - Re: [PATCH v4 2/2] clocksource: Use migrate_disable() to avoid calling get_random

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: <20250203143327.x_ZarOCC@linutronix.de>
Date: Mon, 3 Feb 2025 15:33:27 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Waiman Long <longman@...hat.com>
Cc: John Stultz <jstultz@...gle.com>, Thomas Gleixner <tglx@...utronix.de>,
	Stephen Boyd <sboyd@...nel.org>, Feng Tang <feng.tang@...el.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Clark Williams <clrkwllms@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>, linux-kernel@...r.kernel.org,
	linux-rt-devel@...ts.linux.dev
Subject: Re: [PATCH v4 2/2] clocksource: Use migrate_disable() to avoid
 calling get_random_u32() in atomic context

On 2025-01-31 12:33:23 [-0500], Waiman Long wrote:
> The following bug report happened with a PREEMPT_RT kernel.
> 
> [   30.957705] BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> [   30.957711] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 2012, name: kwatchdog
> [   30.962673] preempt_count: 1, expected: 0
> [   30.962676] RCU nest depth: 0, expected: 0
> 
> It is due to the fact that clocksource_verify_choose_cpus() is invoked
> with preemption disabled.  This function invokes get_random_u32()
> to obtain random numbers for choosing CPUs.  The batched_entropy_32
> local lock and/or the base_crng.lock spinlock in driver/char/random.c
> will be acquired during the call. In PREEMPT_RT kernel, they are both
> sleeping locks and so cannot be acquired in atomic context.
> 
> Fix this problem by using migrate_disable() to allow
> smp_processor_id() to be reliably used without introducing atomic
> context. The preempt_disable() function is then called after
> clocksource_verify_choose_cpus() but before the clocksource measurement
> is being run to avoid introducing unexpected latency.
> 
> Fixes: 7560c02bdffb ("clocksource: Check per-CPU clock synchronization when marked unstable")
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Signed-off-by: Waiman Long <longman@...hat.com>

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>

I would have moved the preempt-disable into the for_each_cpu() loop. But
given that the clocksource is only updated on boot and by the watchdog
this does not matter at runtime.

Sebastian