1. Patchew
  2. linux
  3. View series

[PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access

Breno Leitao posted 1 patch 2 days, 12 hours ago
There is a newer version of this series
net/core/netpoll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Breno Leitao 2 days, 12 hours ago 
In the __netpoll_setup() function, when accessing the device's npinfo
pointer, replace rcu_access_pointer() with rtnl_dereference(). This
change is more appropriate, as suggested by Herbert Xu.

The function is called with the RTNL mutex held, and the pointer is
being dereferenced later, so, dereference earlier and just reuse the
pointer for the if/else.

The replacement ensures correct pointer access while maintaining
the existing locking and RCU semantics of the netpoll subsystem.

Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")
Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 net/core/netpoll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 45fb60bc4803958eb07d4038028269fc0c19622e..30152811e0903a369ccca30500e11e696be158fd 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -626,7 +626,8 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 		goto out;
 	}
 
-	if (!rcu_access_pointer(ndev->npinfo)) {
+	npinfo = rtnl_dereference(ndev->npinfo);
+	if (!npinfo) {
 		npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
 		if (!npinfo) {
 			err = -ENOMEM;
@@ -646,7 +647,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 				goto free_npinfo;
 		}
 	} else {
-		npinfo = rtnl_dereference(ndev->npinfo);
 		refcount_inc(&npinfo->refcnt);
 	}
 

---
base-commit: 66418447d27b7f4c027587582a133dd0bc0a663b
change-id: 20241121-netpoll_rcu_herbet_fix-3f0a433b7860

Best regards,
-- 
Breno Leitao <leitao@debian.org>
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Eric Dumazet 2 days, 3 hours ago 
On Thu, Nov 21, 2024 at 3:59 PM Breno Leitao <leitao@debian.org> wrote:
>
> In the __netpoll_setup() function, when accessing the device's npinfo
> pointer, replace rcu_access_pointer() with rtnl_dereference(). This
> change is more appropriate, as suggested by Herbert Xu.
>
> The function is called with the RTNL mutex held, and the pointer is
> being dereferenced later, so, dereference earlier and just reuse the
> pointer for the if/else.
>
> The replacement ensures correct pointer access while maintaining
> the existing locking and RCU semantics of the netpoll subsystem.
>
> Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")

This seems wrong. This sha1 does not exist, and the title is this patch.

We do not send a patch saying it is fixing itself.

I would suggest instead :

Fixes: c69c5e10adb9 ("netpoll: Use rcu_access_pointer() in __netpoll_setup")

> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  net/core/netpoll.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 45fb60bc4803958eb07d4038028269fc0c19622e..30152811e0903a369ccca30500e11e696be158fd 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -626,7 +626,8 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
>                 goto out;
>         }
>
> -       if (!rcu_access_pointer(ndev->npinfo)) {
> +       npinfo = rtnl_dereference(ndev->npinfo);
> +       if (!npinfo) {
>                 npinfo = kmalloc(sizeof(*npinfo), GFP_KERNEL);
>                 if (!npinfo) {
>                         err = -ENOMEM;
> @@ -646,7 +647,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
>                                 goto free_npinfo;
>                 }
>         } else {
> -               npinfo = rtnl_dereference(ndev->npinfo);
>                 refcount_inc(&npinfo->refcnt);
>         }
>
>
> ---
> base-commit: 66418447d27b7f4c027587582a133dd0bc0a663b
> change-id: 20241121-netpoll_rcu_herbet_fix-3f0a433b7860
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Jakub Kicinski 1 day, 23 hours ago 
On Fri, 22 Nov 2024 00:41:14 +0100 Eric Dumazet wrote:
> > Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")  
> 
> This seems wrong. This sha1 does not exist, and the title is this patch.
> 
> We do not send a patch saying it is fixing itself.
> 
> I would suggest instead :
> 
> Fixes: c69c5e10adb9 ("netpoll: Use rcu_access_pointer() in __netpoll_setup")

Or no Fixes tag and net-next...

I'm missing what can go wrong here, seems like a cleanup.
-- 
pw-bot: cr
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Breno Leitao 1 day, 17 hours ago 
On Thu, Nov 21, 2024 at 07:56:16PM -0800, Jakub Kicinski wrote:
> On Fri, 22 Nov 2024 00:41:14 +0100 Eric Dumazet wrote:
> > > Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")  
> > 
> > This seems wrong. This sha1 does not exist, and the title is this patch.
> > 
> > We do not send a patch saying it is fixing itself.
> > 
> > I would suggest instead :
> > 
> > Fixes: c69c5e10adb9 ("netpoll: Use rcu_access_pointer() in __netpoll_setup")
> 
> Or no Fixes tag and net-next...
> 
> I'm missing what can go wrong here, seems like a cleanup.

I decide to sent to net due Herbert's concern about the previous patch.

I will send a v2 targeting net-next, then.

Thanks for the review,
--breno
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Eric Dumazet 1 day, 17 hours ago 
On Fri, Nov 22, 2024 at 4:56 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 22 Nov 2024 00:41:14 +0100 Eric Dumazet wrote:
> > > Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")
> >
> > This seems wrong. This sha1 does not exist, and the title is this patch.
> >
> > We do not send a patch saying it is fixing itself.
> >
> > I would suggest instead :
> >
> > Fixes: c69c5e10adb9 ("netpoll: Use rcu_access_pointer() in __netpoll_setup")
>
> Or no Fixes tag and net-next...
>
> I'm missing what can go wrong here, seems like a cleanup.

Nothing wrong, perhaps add a Link: next time to avoid confusion

https://lore.kernel.org/lkml/Zz1cKZYt1e7elibV@gondor.apana.org.au/
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Herbert Xu 2 days, 3 hours ago 
On Thu, Nov 21, 2024 at 06:58:58AM -0800, Breno Leitao wrote:
> In the __netpoll_setup() function, when accessing the device's npinfo
> pointer, replace rcu_access_pointer() with rtnl_dereference(). This
> change is more appropriate, as suggested by Herbert Xu.
> 
> The function is called with the RTNL mutex held, and the pointer is
> being dereferenced later, so, dereference earlier and just reuse the
> pointer for the if/else.
> 
> The replacement ensures correct pointer access while maintaining
> the existing locking and RCU semantics of the netpoll subsystem.
> 
> Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  net/core/netpoll.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
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
Re: [PATCH net] netpoll: Use rtnl_dereference() for npinfo pointer access
Posted by Jacob Keller 2 days, 6 hours ago 

On 11/21/2024 6:58 AM, Breno Leitao wrote:
> In the __netpoll_setup() function, when accessing the device's npinfo
> pointer, replace rcu_access_pointer() with rtnl_dereference(). This
> change is more appropriate, as suggested by Herbert Xu.
> 
> The function is called with the RTNL mutex held, and the pointer is
> being dereferenced later, so, dereference earlier and just reuse the
> pointer for the if/else.
> 
> The replacement ensures correct pointer access while maintaining
> the existing locking and RCU semantics of the netpoll subsystem.
> 
> Fixes: c75964e40e69 ("netpoll: Use rtnl_dereference() for npinfo pointer access")
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.