mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] netconsole: fix target enable race and configfs read locking
@ 2026-10-06 18:21 Gustavo Luiz Duarte
  2026-10-06 18:21 ` [PATCH net v2 1/2] netconsole: finish enabling the target before releasing RTNL Gustavo Luiz Duarte
  2026-10-06 18:21 ` [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show() Gustavo Luiz Duarte
  0 siblings, 2 replies; 4+ messages in thread
From: Gustavo Luiz Duarte @ 2026-10-06 18:21 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stephen Hemminger
  Cc: Gustavo Luiz Duarte, netdev, linux-kernel

While looking into the sashiko report about local_ip_show() and
remote_ip_show() missing proper locking [1], I noticed a couple other
places that need locking: patch 1/2 fixes a null-ptr-deref when enabling
a target races against a NETDEV_UNREGISTER event, and patch 2/2 fixes a
use-after-free when the local_mac_show() callback races against a
NETDEV_UNREGISTER event.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814-netcons_ipv6-v3-7-bc0915e8c75f@gmail.com

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
Changes in v2:
- Resending for the 'net' tree.
- Link to v1: https://patch.msgid.link/20260928-netcons-fixes-v1-0-bb5ffe5e698a@gmail.com

To: Breno Leitao <leitao@debian.org>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Gustavo Luiz Duarte (2):
      netconsole: finish enabling the target before releasing RTNL
      netconsole: read np.dev under the RTNL in local_mac_show()

 drivers/net/netconsole.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)
---
base-commit: d5a007b9b457c915ab1a53227e8939e4018aa97a
change-id: 20260925-netcons-fixes-93a3e14326b5

Best regards,
--  
Gustavo Luiz Duarte <gustavold@gmail.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v2 1/2] netconsole: finish enabling the target before releasing RTNL
  2026-10-06 18:21 [PATCH net v2 0/2] netconsole: fix target enable race and configfs read locking Gustavo Luiz Duarte
@ 2026-10-06 18:21 ` Gustavo Luiz Duarte
  2026-10-06 18:21 ` [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show() Gustavo Luiz Duarte
  1 sibling, 0 replies; 4+ messages in thread
From: Gustavo Luiz Duarte @ 2026-10-06 18:21 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stephen Hemminger
  Cc: Gustavo Luiz Duarte, netdev, linux-kernel

netcons_netpoll_setup() publishes nt->np.dev inside __netpoll_setup(),
then releases the RTNL and waits for an RCU grace period before
returning. Only after that does its caller store STATE_ENABLED:

A NETDEV_UNREGISTER event landing in that window tears the target down
and we end up storing STATE_ENABLED with a NULL np->dev, leading to a
null-ptr-deref in netconsole_write():

  BUG: KASAN: null-ptr-deref in netconsole_write+0xb9/0x7f0
  Read of size 8 at addr 00000000000000a8 by task pr/netcon_ext0/135
   netconsole_write+0xb9/0x7f0
   nbcon_emit_next_record+0x501/0x550
   nbcon_emit_one+0x10e/0x170
   nbcon_kthread_func+0x2ff/0x3b0
   kthread+0x199/0x1e0

Store STATE_ENABLED before dropping the RTNL lock to avoid racing with
netconsole_netdev_event().

Fixes: 2382b15bcc39 ("netconsole: take care of NETDEV_UNREGISTER event")
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index b358e5c36735..1fb846ad07d1 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -552,13 +552,16 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
 	err = __netpoll_setup(np, ndev);
 	if (err)
 		goto put;
-	rtnl_unlock();
 
 	/* Make sure all NAPI polls which started before dev->npinfo
 	 * was visible have exited before we start calling NAPI poll.
 	 * NAPI skips locking if dev->npinfo is NULL.
+	 * Hold RTNL until enable is finished so we don't race with
+	 * netconsole_netdev_event()
 	 */
-	synchronize_rcu();
+	synchronize_net();
+	nt->state = STATE_ENABLED;
+	rtnl_unlock();
 
 	return 0;
 
@@ -589,7 +592,6 @@ static void resume_target(struct netconsole_target *nt)
 		return;
 	}
 
-	nt->state = STATE_ENABLED;
 	pr_info("network logging resumed on interface %s\n", nt->np.dev_name);
 }
 
@@ -1067,7 +1069,6 @@ static ssize_t enabled_store(struct config_item *item,
 			goto out_unlock;
 		}
 
-		nt->state = STATE_ENABLED;
 		pr_info("network logging started\n");
 	} else {	/* false */
 		/* We need to disable the netconsole before cleaning it up
@@ -2669,8 +2670,6 @@ static struct netconsole_target *alloc_param_target(char *target_config,
 			 * otherwise, keep the target in the list, but disabled.
 			 */
 			goto fail;
-	} else {
-		nt->state = STATE_ENABLED;
 	}
 	populate_configfs_item(nt, cmdline_count);
 

-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show()
  2026-10-06 18:21 [PATCH net v2 0/2] netconsole: fix target enable race and configfs read locking Gustavo Luiz Duarte
  2026-10-06 18:21 ` [PATCH net v2 1/2] netconsole: finish enabling the target before releasing RTNL Gustavo Luiz Duarte
@ 2026-10-06 18:21 ` Gustavo Luiz Duarte
  2026-10-07  8:58   ` Breno Leitao
  1 sibling, 1 reply; 4+ messages in thread
From: Gustavo Luiz Duarte @ 2026-10-06 18:21 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Stephen Hemminger
  Cc: Gustavo Luiz Duarte, netdev, linux-kernel

local_mac_show() loads nt->np.dev and dereferences it with no lock. It
can race with the interface going away and netconsole_netdev_event()
eventually calling netdev_put() and freeing the device before we
dereference it, leading to a UAF.

The window is only a few instructions wide, so I could only reproduce it
with an artificially widened window between the load and the
dereference:

  BUG: KASAN: slab-use-after-free in local_mac_show+0x4e/0x80
  Read of size 8 at addr ffff88800e990458 by task uaf/130
   local_mac_show+0x4e/0x80
   configfs_read_iter+0x187/0x260
   vfs_read+0x453/0x5a0

  Allocated by task 116:
   alloc_netdev_mqs+0x78/0x830
   rtnl_create_link+0x53b/0x5c0

  Freed by task 121:
   kfree+0x159/0x420
   device_release+0x77/0x120
   kobject_put+0xb5/0x160
   netdev_run_todo+0x420/0x850
   rtnl_dellink+0x259/0x5b0

Fixes: 0953864160bd ("[NETPOLL]: no need to store local_mac")
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 1fb846ad07d1..830262864da2 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -888,10 +888,17 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
 
 static ssize_t local_mac_show(struct config_item *item, char *buf)
 {
-	struct net_device *dev = to_target(item)->np.dev;
 	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+	struct netconsole_target *nt = to_target(item);
+	int ret;
 
-	return sysfs_emit(buf, "%pM\n", dev ? dev->dev_addr : bcast);
+	/* Hold RTNL here so netconsole_netdev_event() doesn't tear down np.dev
+	 * from under us.
+	 */
+	rtnl_lock();
+	ret = sysfs_emit(buf, "%pM\n", nt->np.dev ? nt->np.dev->dev_addr : bcast);
+	rtnl_unlock();
+	return ret;
 }
 
 static ssize_t remote_mac_show(struct config_item *item, char *buf)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show()
  2026-10-06 18:21 ` [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show() Gustavo Luiz Duarte
@ 2026-10-07  8:58   ` Breno Leitao
  0 siblings, 0 replies; 4+ messages in thread
From: Breno Leitao @ 2026-10-07  8:58 UTC (permalink / raw)
  To: Gustavo Luiz Duarte
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Stephen Hemminger, netdev, linux-kernel

Hello Gustavo,

On Tue, Oct 06, 2026 at 07:21:26PM +0100, Gustavo Luiz Duarte wrote:
> local_mac_show() loads nt->np.dev and dereferences it with no lock. It
> can race with the interface going away and netconsole_netdev_event()
> eventually calling netdev_put() and freeing the device before we
> dereference it, leading to a UAF.

Thanks for the fix.

> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -888,10 +888,17 @@ static ssize_t remote_ip_show(struct config_item *item, char *buf)
>  
>  static ssize_t local_mac_show(struct config_item *item, char *buf)
>  {
> -	struct net_device *dev = to_target(item)->np.dev;
>  	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
> +	struct netconsole_target *nt = to_target(item);
> +	int ret;
>  
> -	return sysfs_emit(buf, "%pM\n", dev ? dev->dev_addr : bcast);
> +	/* Hold RTNL here so netconsole_netdev_event() doesn't tear down np.dev
> +	 * from under us.
> +	 */
> +	rtnl_lock();

Same idea as Eric suggested earlier, this might be doable without having
the heavy RTNL, but using netconsole's locks

--breno

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-07  8:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-06 18:21 [PATCH net v2 0/2] netconsole: fix target enable race and configfs read locking Gustavo Luiz Duarte
2026-10-06 18:21 ` [PATCH net v2 1/2] netconsole: finish enabling the target before releasing RTNL Gustavo Luiz Duarte
2026-10-06 18:21 ` [PATCH net v2 2/2] netconsole: read np.dev under the RTNL in local_mac_show() Gustavo Luiz Duarte
2026-10-07  8:58   ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®