[PATCH] netdevsim: fix use-after-free in __nsim_dev_port_del

[PATCH] netdevsim: fix use-after-free in __nsim_dev_port_del

[Hrushiraj Gandhi]

debugfs files created under a port's ddir (ethtool/get_err,
ethtool/set_err, ring params, bpf_offloaded_id, udp_ports/inject_error,
etc.) store raw pointers directly into the netdevsim struct, which lives
in the net_device private data kmalloc slab.

If these files outlive 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 debugfs_u32_get().

In __nsim_dev_port_del(), nsim_destroy() is called before
nsim_dev_port_debugfs_exit(). However, nsim_destroy() calls free_netdev()
at its end, while nsim_dev_port_debugfs_exit() removes the port's
debugfs directory. This means the slab is freed before the debugfs
files are removed.

Fix by calling debugfs_remove_recursive(ns->nsim_dev_port->ddir) in
nsim_destroy() right before free_netdev(). This ensures all per-port
debugfs files are destroyed synchronously before the backing memory is
freed. The subsequent call to nsim_dev_port_debugfs_exit() in
__nsim_dev_port_del() becomes a harmless no-op.

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>
---
 drivers/net/netdevsim/netdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 27e5f109f933..08136e7990cb 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -1214,6 +1214,13 @@ void nsim_destroy(struct netdevsim *ns)
 		ns->page = NULL;
 	}
 
+	/*
+	 * Remove per-port debugfs files before free_netdev() releases the
+	 * netdevsim struct to prevent use-after-free in concurrent readers.
+	 */
+	debugfs_remove_recursive(ns->nsim_dev_port->ddir);
+	ns->nsim_dev_port->ddir = NULL;
+
 	free_netdev(dev);
 }
 
-- 
2.47.3

Re: [PATCH] netdevsim: fix use-after-free in __nsim_dev_port_del

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
Subject: netdevsim: fix use-after-free in __nsim_dev_port_del

The patch removes the per-port debugfs directory in nsim_destroy()
before free_netdev() so that debugfs files referencing fields inside
the netdevsim struct cannot be reached after the slab is released.

Link: https://syzkaller.appspot.com/bug?extid=6c25f4750230faf70be9

> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index 27e5f109f933..08136e7990cb 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -1214,6 +1214,13 @@ void nsim_destroy(struct netdevsim *ns)
>  		ns->page = NULL;
>  	}
>  
> +	/*
> +	 * Remove per-port debugfs files before free_netdev() releases the
> +	 * netdevsim struct to prevent use-after-free in concurrent readers.
> +	 */
> +	debugfs_remove_recursive(ns->nsim_dev_port->ddir);
> +	ns->nsim_dev_port->ddir = NULL;
> +
>  	free_netdev(dev);
>  }

[High]
Does the same use-after-free window remain on the nsim_create() error
path?

nsim_create() unconditionally calls nsim_ethtool_init(ns) before the
init step that can fail:

    SET_NETDEV_DEVLINK_PORT(dev, &nsim_dev_port->devlink_port);
    nsim_ethtool_init(ns);
    if (nsim_dev_port_is_pf(nsim_dev_port))
        err = nsim_init_netdevsim(ns);
    else
        err = nsim_init_netdevsim_vf(ns);
    if (err)
        goto err_free_netdev;
    ...
err_free_netdev:
    free_netdev(dev);
    return ERR_PTR(err);

nsim_ethtool_init() populates files under nsim_dev_port->ddir whose
data pointers reference fields inside ns, for example:

    ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
    debugfs_create_u32("get_err", 0600, ethtool, &ns->ethtool.get_err);
    debugfs_create_u32("set_err", 0600, ethtool, &ns->ethtool.set_err);
    ...
    dir = debugfs_create_dir("ring", ethtool);
    debugfs_create_u32("rx_max_pending", 0600, dir,
                      &ns->ethtool.ring.rx_max_pending);

If nsim_init_netdevsim() (or nsim_init_netdevsim_vf()) fails, the
err_free_netdev label runs free_netdev(dev) directly and returns
ERR_PTR(err). The caller then calls nsim_dev_port_debugfs_exit() to
tear down ddir, but in the meantime the netdevsim slab has already
been freed while these debugfs files are still live with data pointers
into it.

Should the same debugfs_remove_recursive(nsim_dev_port->ddir) be done
on the err_free_netdev path before free_netdev(dev), or should the
debugfs file creation be moved past the point of no return so the
fuzzer-reachable failure paths (alloc pressure on mock_phc_create,
register_netdevice, nsim_psp_init, etc.) cannot leave dangling
debugfs entries?

The Fixes: tag e05b2d141fef appears to cover this path as well.