The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is
being accessed directly for a NULL check. While no RCU read lock is held
in this context, we should still use proper RCU primitives for
consistency and correctness.

Replace the direct NULL check with rcu_access_pointer(), which is the
appropriate primitive when only checking for NULL without dereferencing
the pointer. This function provides the necessary ordering guarantees
without requiring RCU read-side protection.

Signed-off-by: Breno Leitao <leitao@debian.org>
Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.")
---
 include/linux/netpoll.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index cd4e28db0cbd77572a579aff2067b5864d1a904a..959a4daacea1f2f76536e309d198bc14407942a4 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -72,7 +72,7 @@ static inline void *netpoll_poll_lock(struct napi_struct *napi)
 {
 	struct net_device *dev = napi->dev;
 
-	if (dev && dev->npinfo) {
+	if (dev && rcu_access_pointer(dev->npinfo)) {
 		int owner = smp_processor_id();
 
 		while (cmpxchg(&napi->poll_owner, -1, owner) != -1)

-- 
2.43.5

On Mon, Nov 18, 2024 at 03:15:18AM -0800, Breno Leitao wrote:
>
> diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> index cd4e28db0cbd77572a579aff2067b5864d1a904a..959a4daacea1f2f76536e309d198bc14407942a4 100644
> --- a/include/linux/netpoll.h
> +++ b/include/linux/netpoll.h
> @@ -72,7 +72,7 @@ static inline void *netpoll_poll_lock(struct napi_struct *napi)
>  {
>  	struct net_device *dev = napi->dev;
>  
> -	if (dev && dev->npinfo) {
> +	if (dev && rcu_access_pointer(dev->npinfo)) {

This function is only ever called in a BH context and as such
the correct primitive would be rcu_dereference.  Indeed calling
this outside of BH/RCU context would be silly.

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

On Mon, Nov 18, 2024 at 03:15:18AM -0800, Breno Leitao wrote:
> The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is
> being accessed directly for a NULL check. While no RCU read lock is held
> in this context, we should still use proper RCU primitives for
> consistency and correctness.
> 
> Replace the direct NULL check with rcu_access_pointer(), which is the
> appropriate primitive when only checking for NULL without dereferencing
> the pointer. This function provides the necessary ordering guarantees
> without requiring RCU read-side protection.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.")

nitpick: As for the first patch - please check the tags order.

Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>

On Mon, Nov 18, 2024 at 01:20:30PM +0100, Michal Kubiak wrote:
> On Mon, Nov 18, 2024 at 03:15:18AM -0800, Breno Leitao wrote:
> > The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is
> > being accessed directly for a NULL check. While no RCU read lock is held
> > in this context, we should still use proper RCU primitives for
> > consistency and correctness.
> > 
> > Replace the direct NULL check with rcu_access_pointer(), which is the
> > appropriate primitive when only checking for NULL without dereferencing
> > the pointer. This function provides the necessary ordering guarantees
> > without requiring RCU read-side protection.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.")
> 
> nitpick: As for the first patch - please check the tags order.
> 
> Thanks,
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>

Thanks for the review.

I am not planning to resend it now, since I think maintainer's tooling will
reorder that.

If that is not true, I am more than happy to resend fixing the order.

Thanks
Breno

On Mon, 18 Nov 2024 07:37:43 -0800 Breno Leitao wrote:
> I am not planning to resend it now, since I think maintainer's tooling will
> reorder that.

FWIW ours doesn't, but I fixed up manually.