Forwarded: [PATCH v3] netdevsim: fix use-after-free of ethtool debugfs data

[syzbot <syzbot+6c25f4750230faf70be9@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH v3] netdevsim: fix use-after-free of ethtool debugfs data
Author: hrushirajg23@gmail.com

From: Hrushiraj Gandhi <hrushirajg23@gmail.com>

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git main

debugfs files created by nsim_ethtool_init() store raw pointers
directly into the netdevsim struct, which lives in the net_device
private data kmalloc slab.

If the ethtool subdirectory outlives the netdevsim struct, a concurrent
reader can trigger a slab-use-after-free by passing debugfs_file_get()
(which only checks dentry lifetime) and then dereferencing the freed
data pointer in e.g. debugfs_u32_get().

In __nsim_dev_port_del(), nsim_destroy() is called before
nsim_dev_port_debugfs_exit(). nsim_destroy() ends with free_netdev(),
while nsim_dev_port_debugfs_exit() removes the port's debugfs directory
afterwards. This means the slab is freed before the ethtool debugfs
files that point into it are removed.

The same window exists on nsim_create()'s error path: nsim_ethtool_init()
creates debugfs files under ddir with pointers into ns before
nsim_init_netdevsim()/nsim_init_netdevsim_vf(), which can fail. The
err_free_netdev label then calls free_netdev() while those ethtool
debugfs entries are still live.

All other features that create per-port debugfs files (pp_hold,
queue_reset, vlan) already remove their own entries explicitly in
nsim_destroy() before free_netdev(). ethtool is the only one that does
not. Fix this by saving the ethtool dentry in struct nsim_ethtool and
calling debugfs_remove_recursive() on it before free_netdev() in both
nsim_destroy() and the nsim_create() error path. The port ddir teardown
is left to nsim_dev_port_debugfs_exit() as before.

Reported-by: syzbot+6c25f4750230faf70be9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6c25f4750230faf70be9
Fixes: e05b2d141fef ("netdevsim: move netdev creation/destruction to dev probe")
Signed-off-by: Hrushiraj Gandhi <hrushirajg23@gmail.com>
---
v3:
- Instead of removing the entire port ddir before free_netdev(), save
  and remove only the ethtool dentry. All other features (pp_hold,
  queue_reset, vlan) already clean up after themselves; ethtool is the
  only exception. This aligns ethtool with how the other features
  behave, as suggested by Jakub Kicinski.
v2:
- Also fix the same use-after-free window on the error path of
  nsim_create() as suggested by Simon Horman.
- Shorten the code comment in nsim_destroy() to be more concise.
---
 drivers/net/netdevsim/ethtool.c   | 1 +
 drivers/net/netdevsim/netdev.c    | 9 +++++++++
 drivers/net/netdevsim/netdevsim.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 9350ba48eb81..a465f3220a7c 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -252,6 +252,7 @@ void nsim_ethtool_init(struct netdevsim *ns)
 	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
 
 	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
+	ns->ethtool.ddir = ethtool;
 
 	debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
 	debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 27e5f109f933..09d86a591fac 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -1165,6 +1165,7 @@ struct netdevsim *nsim_create(struct nsim_dev *nsim_dev,
 	return ns;
 
 err_free_netdev:
+	debugfs_remove_recursive(ns->ethtool.ddir);
 	free_netdev(dev);
 	return ERR_PTR(err);
 }
@@ -1214,6 +1215,14 @@ void nsim_destroy(struct netdevsim *ns)
 		ns->page = NULL;
 	}
 
+	/*
+	 * Remove the ethtool debugfs directory before free_netdev() releases
+	 * the netdevsim struct to prevent use-after-free in concurrent readers.
+	 * Other per-port debugfs files are already removed above, and the port
+	 * ddir itself is cleaned up by nsim_dev_port_debugfs_exit().
+	 */
+	debugfs_remove_recursive(ns->ethtool.ddir);
+
 	free_netdev(dev);
 }
 
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 4c9cc96dcec3..c08a01af31e9 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -95,6 +95,7 @@ struct nsim_ethtool {
 	struct ethtool_coalesce coalesce;
 	struct ethtool_ringparam ring;
 	struct ethtool_fecparam fec;
+	struct dentry *ddir;
 };
 
 struct nsim_rq {
-- 
2.47.3