linux-kernel - Re: [PATCH] tracing: fprobe: fix suspicious rcu usage in fprobe

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: <aLVwDR_NJSAGaHdf@gondor.apana.org.au>
Date: Mon, 1 Sep 2025 18:06:05 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: paulmck@...nel.org
Cc: rostedt@...dmis.org, dongml2@...natelecom.cn, mhiramat@...nel.org,
	mathieu.desnoyers@...icios.com, linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, oliver.sang@...el.com,
	tgraf@...g.ch, linux-crypto@...r.kernel.org
Subject: Re: [PATCH] tracing: fprobe: fix suspicious rcu usage in fprobe_entry

Paul E. McKenney <paulmck@...nel.org> wrote:
>
> Yes, preempt_disable() does indeed start an RCU read-side critical section,
> just as surely as rcu_read_lock() does.
> 
> However, this is a lockdep check inside of __rhashtable_lookup():
> 
>        rht_dereference_rcu(ht->tbl, ht)
> 
> Which is defined as:
> 
>        rcu_dereference_check(p, lockdep_rht_mutex_is_held(ht));
> 
> This is explicitly telling lockdep that rcu_read_lock() is OK and
> holding ht->mutex is OK, but nothing else is.

I think that's a deficiency in rcu_dereference_check.

Yes I could certainly add a preemption check to rht_dereference_rcu,
but that makes zero sense because this is an implementation detail
of RCU and there is no reason why this logic should be added to
rhashtable.

rhashtable never relies on the fact that turning off preemption
creates is safe for RCU reads, so it makes no sense to add this
logic to rht_dereference_rcu since RCU could conceivably (even
if it is unlikely) be changed on day so that turning off preemption
is no longer safe for RCU reads.

My preference would be to add the preemption test to
rcu_dereference_check, or if that is not possible for some reason,
create a new RCU helper that includes the preemption test.

Of course just adding RCU read locks as this patch does is also
fine.

Thanks,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt