Re: [PATCH net v2] netdevsim: fib: fix use-after-free of FIB data via debugfs

[Ido Schimmel]

On Fri, May 29, 2026 at 06:57:17AM -0700, Zijing Yin wrote:
> netdevsim: fib: fix use-after-free of FIB data via debugfs
> 
> Writing to the netdevsim debugfs file
> "netdevsim/netdevsimN/fib/nexthop_bucket_activity" enters
> nsim_nexthop_bucket_activity_write(), which looks up a nexthop in
> data->nexthop_ht under rtnl_lock(). If a network namespace teardown,
> devlink reload or device deletion runs concurrently, nsim_fib_destroy()
> frees that rhashtable (and the surrounding nsim_fib_data) while the
> write is still in flight, leading to a slab-use-after-free:

[...]

> The freed 1k object is the bucket table of data->nexthop_ht. Shortly
> after, the dangling table is dereferenced again and the machine also
> takes a GPF in __rht_bucket_nested() from the same call site.
> 
> The root cause is a lifetime mismatch: the debugfs files reference
> nsim_fib_data (the writer dereferences data->nexthop_ht), but the
> interface is not bracketed around the lifetime of that data.
> nsim_fib_destroy() freed both rhashtables and only removed the debugfs
> directory afterwards, and nsim_fib_create() created the debugfs files
> before the rhashtables were initialized and, on the error path, freed
> them before removing the files. debugfs keeps the file itself alive
> across a ->write() via debugfs_file_get()/debugfs_file_put()
> (fs/debugfs/file.c), but it does not keep data->nexthop_ht alive, so the
> in-flight writer dereferenced freed memory. rtnl_lock() in the writer
> does not help, because the teardown path does not take rtnl around
> rhashtable_free_and_destroy().
> 
> Fix it by bracketing the debugfs interface around the data it exposes,
> keeping nsim_fib_create() and nsim_fib_destroy() symmetric:
> 
>  - In nsim_fib_destroy(), tear down the debugfs files before the data
>    structures they reference. debugfs_remove_recursive() drops the
>    initial active-user reference and then waits for every in-flight
>    ->write() to drop its reference before returning, and rejects new
>    opens (__debugfs_file_removed(), fs/debugfs/inode.c). Once it returns,
>    no debugfs accessor can reach the FIB data, so the rhashtables and
>    nsim_fib_data can be destroyed safely. This also covers the bool knobs
>    in the same directory, which store pointers into the same
>    nsim_fib_data, and the final kfree(data).
> 
>  - In nsim_fib_create(), create the debugfs files after the rhashtables
>    and notifiers are set up. This closes the same race on the
>    error-unwind path, where a concurrent writer could otherwise observe a
>    half-constructed instance or a table that the unwind has already
>    freed. (With only the destroy-side change, a writer racing the create
>    window instead dereferences an uninitialized data->nexthop_ht.)
> 
> This is reproducible by racing, in a loop, writes to
> /sys/kernel/debug/netdevsim/netdevsimN/fib/nexthop_bucket_activity
> against a teardown of the same netdevsim instance -- a devlink reload
> ("devlink dev reload netdevsim/netdevsimN"), destroying the network
> namespace it lives in, or "echo N > /sys/bus/netdevsim/del_device". It
> was found with syzkaller; a syzkaller reproducer is available. A
> standalone C reproducer does not trigger it reliably because the race
> needs the netns-teardown/reload path.
> 
> Fixes: c6385c0b67c5 ("netdevsim: Allow reporting activity on nexthop buckets")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Zijing Yin <yzjaurora@xxxxxxxxx>

Reviewed-by: Ido Schimmel <idosch@xxxxxxxxxx>