linux-kernel - Re: [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release in attach()

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: <da41cb80e0acb08fd25c38876a29156b69322249.camel@sipsolutions.net>
Date: Fri, 25 Apr 2025 12:22:46 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Matthias Brugger
	 <matthias.bgg@...il.com>, AngeloGioacchino Del Regno
	 <angelogioacchino.delregno@...labora.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next] net: ethernet: mtk_wed: annotate RCU release
 in attach()

On Fri, 2025-04-25 at 11:20 +0100, Simon Horman wrote:
> On Wed, Apr 23, 2025 at 05:08:08PM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@...el.com>
> > 
> > There are some sparse warnings in wifi, and it seems that
> > it's actually possible to annotate a function pointer with
> > __releases(), making the sparse warnings go away. In a way
> > that also serves as documentation that rcu_read_unlock()
> > must be called in the attach method, so add that annotation.
> > 
> > Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> 
> Reviewed-by: Simon Horman <horms@...nel.org>

Thanks :) Looks like Jakub already applied it, FWIW.

> Thinking out loud:
> 
> * Without this patch I see the following, but with this patch I do not.
> 
>   .../mt7915/mmio.c:636:5: warning: context imbalance in 'mt7915_mmio_wed_init' - wrong count at exit
>   .../mt7996/mmio.c:302:5: warning: context imbalance in 'mt7996_mmio_wed_init' - wrong count at exit

Right, that's what I was trying to get rid of.

> * The only implementation of this callback I found is mtk_wed_attach
>   which is already annotated as __releases(RCU);

Indeed, but sparse doesn't check that for function pointer
compatibility.

> * The only caller of this callback I could find is mtk_wed_device_attach()
>   which takes rcu_read_unlock(). And the the callback needs to release it
>   to avoid imbalance.
> 

Right, pretty sure that's all intentional (though I don't understand
it), hence this change to document it a bit better and get rid of the
warnings.

johannes