Netdev List
 help / color / mirror / Atom feed
* [PATCH net] netdevsim: delete ports added after a failed reload
@ 2026-10-29  3:02 Joas Antonio dos Santos
  2026-10-30  6:49 ` Mina Almasry
  2026-11-02  3:16 ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Joas Antonio dos Santos @ 2026-10-29  3:02 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, syzbot+88435e204734072bf39a, netdev, linux-kernel

When a devlink reload fails, nsim_dev_reload_down() has already torn
the device down through nsim_dev_reload_destroy(), and
nsim_dev_reload_destroy() returns early on every later call because
devlink_is_reload_failed() is set.  new_port still works in that state
though: nsim_drv_port_add() does not look at the reload state and adds
the port to nsim_dev->port_list.  Nothing ever deletes it.  When the
device is removed, nsim_drv_remove() destroys the BPF offload device
and frees the devlink instance while the port's netdev is still
registered:

  WARNING: kernel/bpf/offload.c:794 at bpf_offload_dev_destroy+0x50/0x78
   nsim_bpf_dev_exit+0x7c/0xbc
  WARNING: net/devlink/core.c:557 at devlink_free+0x30c/0x3ec

and the leftover netdev then points to freed memory.  Bringing it up
afterwards:

  BUG: KASAN: slab-use-after-free in devlink_bus_name+0xa0/0xac
  Read of size 8 at addr fff000001a313118 by task ip/120
   devlink_bus_name+0xa0/0xac
   devlink_nl_port_handle_size+0x38/0x80
   if_nlmsg_size+0x340/0x7a8
   rtmsg_ifinfo_build_skb+0x74/0x218
   rtmsg_ifinfo+0xa0/0x18c
   __dev_notify_flags+0x1d8/0x2f4
   netif_change_flags+0xe0/0x150
   dev_change_flags+0x8c/0x1e4
   devinet_ioctl+0x9c0/0x1360

A successful reload after the failed one has the same problem:
nsim_dev_reload_destroy() skips the ports again and
nsim_dev_reload_create() then reinitializes port_list over them.

The ports are the only thing that can be created after the failed
reload, so delete them on the early-return path too.  The port_list is
valid there: the failed reload emptied it with nsim_dev_port_del_all()
and nsim_dev_reload_up() fails before nsim_dev_reload_create().

Fixes: 75ba029f3c07 ("netdevsim: implement proper devlink reload")
Reported-by: syzbot+88435e204734072bf39a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=88435e204734072bf39a
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
Tested on net/main 94cea342e891 (arm64, QEMU, generic KASAN) with the
syzbot C reproducer (sets fail_reload, does a devlink reload, adds a
port with new_port, then del_device), followed by "ip link set <dev>
up" on any netdev left behind.  Without this patch both WARNINGs above
fire, the port's netdev survives del_device, and bringing it up gives
the KASAN report above (2 of 2 boots).  With it, 3 of 3 boots show no
WARNING or KASAN report and no netdev is left behind.
drivers/net/netdevsim/dev.o builds cleanly with W=1.  The
reload-after-failed-reload case was not tested separately.

 drivers/net/netdevsim/dev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index feb88cc..a871790 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1697,8 +1697,13 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 
-	if (devlink_is_reload_failed(devlink))
+	if (devlink_is_reload_failed(devlink)) {
+		/* The failed reload already tore everything down, but ports
+		 * can still be added through new_port after that.
+		 */
+		nsim_dev_port_del_all(nsim_dev);
 		return;
+	}
 	debugfs_remove(nsim_dev->take_snapshot);
 
 	if (nsim_dev_get_vfs(nsim_dev)) {
-- 
2.39.5

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

* Re: [PATCH net] netdevsim: delete ports added after a failed reload
  2026-10-29  3:02 [PATCH net] netdevsim: delete ports added after a failed reload Joas Antonio dos Santos
@ 2026-10-30  6:49 ` Mina Almasry
  2026-11-02  3:16 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Mina Almasry @ 2026-10-30  6:49 UTC (permalink / raw)
  To: Joas Antonio dos Santos
  Cc: Mina Almasry, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman,
	syzbot+88435e204734072bf39a, netdev, linux-kernel

On Wed, Oct 28, 2026 at 8:02 PM Joas Antonio dos Santos
<joasantonio108@gmail.com> wrote:
>
> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index feb88cc..a871790 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -1697,8 +1697,13 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
>  {
>         struct devlink *devlink = priv_to_devlink(nsim_dev);
>
> -       if (devlink_is_reload_failed(devlink))
> +       if (devlink_is_reload_failed(devlink)) {
> +               /* The failed reload already tore everything down, but ports
> +                * can still be added through new_port after that.
> +                */
> +               nsim_dev_port_del_all(nsim_dev);
>                 return;
> +       }

I prefer Ömer's version for the reasons mentioned here:
https://lore.kernel.org/netdev/20261030064245.4118081-1-almasrymina@google.com/

Either way, since this is a non-critical fix in netdevsim, it should
target net-next once it reopens after the merge window:
https://netdev.bots.linux.dev/net-next.html

-- 
Thanks,
Mina

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

* Re: [PATCH net] netdevsim: delete ports added after a failed reload
  2026-10-29  3:02 [PATCH net] netdevsim: delete ports added after a failed reload Joas Antonio dos Santos
  2026-10-30  6:49 ` Mina Almasry
@ 2026-11-02  3:16 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-02  3:16 UTC (permalink / raw)
  To: joasantonio108
  Cc: kuba, andrew+netdev, davem, edumazet, pabeni, horms,
	syzbot+88435e204734072bf39a, netdev, linux-kernel

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The fix is incomplete, and the claim it rests on is wrong.

--

netdevsim: delete ports added after a failed reload

This patch makes nsim_dev_reload_destroy() call nsim_dev_port_del_all() on
the early-return path that is taken after a failed devlink reload. Before
this, ports added through new_port after the failed reload were never
deleted, which led to WARNs in devlink_free() and a use-after-free on the
leftover netdev.

Closes: https://syzkaller.appspot.com/bug?extid=88435e204734072bf39a

> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index feb88ccbbcedb..a8717908cfb83 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -1697,8 +1697,13 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
>  {
>  	struct devlink *devlink = priv_to_devlink(nsim_dev);
>  
> -	if (devlink_is_reload_failed(devlink))
> +	if (devlink_is_reload_failed(devlink)) {
> +		/* The failed reload already tore everything down, but ports
> +		 * can still be added through new_port after that.
> +		 */
> +		nsim_dev_port_del_all(nsim_dev);
>  		return;
> +	}

[Severity: Medium]
Is deleting the ports enough on this path? The commit message says:

  "The ports are the only thing that can be created after the failed
  reload, so delete them on the early-return path too."

After a failed reload, user space can apparently still set sriov_numvfs,
switch the eswitch to switchdev mode, and create devlink rate nodes. None
of these paths check devlink_is_reload_failed():

  nsim_bus_dev_numvfs_store()
    nsim_drv_configure_vfs()
      nsim_bus_dev_set_vfs(nsim_bus_dev, num_vfs);
      nsim_esw_switchdev_enable()

  nsim_devlink_eswitch_mode_set()
    nsim_esw_switchdev_enable()
      __nsim_dev_port_add(nsim_dev, NSIM_DEV_PORT_TYPE_VF, i, NULL);
      nsim_dev->esw_mode = DEVLINK_ESWITCH_MODE_SWITCHDEV;

  devlink_nl_rate_new_doit()
    nsim_rate_node_new()
      if (!nsim_esw_mode_is_switchdev(nsim_dev)) {
          ...
          return -EOPNOTSUPP;
      }

Before deleting ports, the normal path further down in
nsim_dev_reload_destroy() resets the VFs and tears down the eswitch state:

	if (nsim_dev_get_vfs(nsim_dev)) {
		nsim_bus_dev_set_vfs(nsim_dev->nsim_bus_dev, 0);
		if (nsim_esw_mode_is_switchdev(nsim_dev))
			nsim_esw_legacy_enable(nsim_dev, NULL);
	}

nsim_esw_legacy_enable() is the only caller of devl_rate_nodes_destroy() in
netdevsim. The new early-return branch never reaches it.

Take this sequence: fail_reload=1, reload fails, set sriov_numvfs, switch to
switchdev, run "devlink port function rate add <dev>/node1", then
del_device. The early branch removes the ports but leaves the rate node on
devlink->rate_list. Would devlink_free() then hit this check:

net/devlink/core.c:devlink_free() {
    ...
	WARN_ON(devlink_rates_check(devlink, NULL, NULL));
    ...
}

and leak the devlink_rate and the nsim_rate_node?

The commit message also says a successful reload after the failed one is
fixed. In that case the early branch deletes the VF ports but leaves
num_vfs set and esw_mode as switchdev. Any remaining rate node would trip
WARN_ON(devlink_rates_check(...)) in devlink_reload_reinit_sanity_check().

After that, nsim_dev_reload_create() re-adds only the PF ports. The device
then ends up in switchdev mode with VFs configured but no VF representor
ports. A normal reload never leaves the device in that state.

Should the failed branch do the same VF reset and nsim_esw_legacy_enable()
teardown as the normal path before calling nsim_dev_port_del_all()? If not,
the "only thing that can be created" sentence in the commit message should
probably be corrected.

>  	debugfs_remove(nsim_dev->take_snapshot);
>  
>  	if (nsim_dev_get_vfs(nsim_dev)) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179324296598.34505.11070163314749859668%40gmail.com

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

end of thread, other threads:[~2026-11-02  3:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29  3:02 [PATCH net] netdevsim: delete ports added after a failed reload Joas Antonio dos Santos
2026-10-30  6:49 ` Mina Almasry
2026-11-02  3:16 ` netdev-bot+sashiko

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