netdev - Re: [PATCH net v3 1/3] netpoll: fix incorrect refcount handling causing incorrect cleanup

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: <5hiawr3ved4xonlmc3yxmomhimn3lo5d2u7z66elkyurcde2nq@7kqossblfmfy>
Date: Wed, 10 Sep 2025 07:07:08 -0700
From: Breno Leitao <leitao@...ian.org>
To: Jay Vosburgh <jv@...sburgh.net>
Cc: Jakub Kicinski <kuba@...nel.org>, Calvin Owens <calvin@...nvd.org>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Shuah Khan <shuah@...nel.org>, 
	Simon Horman <horms@...nel.org>, david decotigny <decot@...glers.com>, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	asantostc@...il.com, efault@....de, kernel-team@...a.com, stable@...r.kernel.org
Subject: Re: [PATCH net v3 1/3] netpoll: fix incorrect refcount handling
 causing incorrect cleanup

Hello Jay,

On Tue, Sep 09, 2025 at 05:18:26PM -0700, Jay Vosburgh wrote:
> 	Second, I'm a bit unsure what's going on with the struct netpoll
> *np parameter of __netpoll_setup for the second and subsequent netpoll
> instances (i.e., second and later call), as the function will
> unconditionally do
> 
> 	npinfo->netpoll = np;
> 
> 	which it seems like would overwrite the "np" supplied by any
> prior calls to __netpoll_setup. 

This is clearly a bug. Trying to understand where this field is used, it
seems it is not used at all.

I was not able to find any usage of it, and removing it and compiling
with `allyesconfig` didn't complain about any other users also.

I think we should remove it.

commit 5da5611575ce94ca557c194d4147ae3011cedb6f
Author: Breno Leitao <leitao@...ian.org>
Date:   Wed Sep 10 06:32:25 2025 -0700

    netpoll: remove unused netpoll pointer from netpoll_info
    
    The netpoll_info structure contained a useless pointer back to its
    associated netpoll. This field is never used, and the assignment in
    __netpoll_setup() is does not comtemplate multiple instances, as
    reported by Jay[1].
    
    Drop both the member and its initialization to simplify the structure.
    
    Link: https://lore.kernel.org/all/2930648.1757463506@famine/ [1]
    Reported-by: Jay Vosburgh <jv@...sburgh.net>
    Signed-off-by: Breno Leitao <leitao@...ian.org>

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index b5ea9882eda8b..f22eec4660405 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -55,7 +55,6 @@ struct netpoll_info {
 
 	struct delayed_work tx_work;
 
-	struct netpoll *netpoll;
 	struct rcu_head rcu;
 };
 
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 5f65b62346d4e..c58faa7471650 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -591,7 +591,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
 
 	np->dev = ndev;
 	strscpy(np->dev_name, ndev->name, IFNAMSIZ);
-	npinfo->netpoll = np;
 
 	/* fill up the skb queue */
 	refill_skbs(np);