mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] netdevsim: check the driver is bound in new_port/del_port
@ 2026-11-01 13:58 Nguyen Ngoc Thang
  2026-11-02  2:19 ` Hangbin Liu
  2026-11-09  1:18 ` Hangbin Liu
  0 siblings, 2 replies; 3+ messages in thread
From: Nguyen Ngoc Thang @ 2026-11-01 13:58 UTC (permalink / raw)
  To: kuba, andrew+netdev, davem, edumazet, pabeni
  Cc: netdev, linux-kernel, syzbot+b3fca27eb4005f0111f4, Nguyen Ngoc Thang

new_port and del_port are attributes of the bus device, so they stay
reachable after the driver is unbound. nsim_drv_remove() clears
drvdata, yet both handlers pass it straight to priv_to_devlink() and
devl_lock(), which faults on a small negative pointer:

  BUG: unable to handle page fault for address: fffffffffffffeb0
  RIP: 0010:__lock_acquire+0x41/0x2de0 kernel/locking/lockdep.c:5140
  Call Trace:
   lock_acquire+0x115/0x350
   mutex_lock_nested+0x55/0x1d0
   nsim_drv_port_add+0x5a/0x140 drivers/net/netdevsim/dev.c:1760
   new_port_store+0x1a5/0x280 drivers/net/netdevsim/bus.c:96
   kernfs_fop_write_iter+0x3a5/0x540

The nsim_bus_dev->init check does not help: it is only cleared when
the whole device is deleted, not on unbind.

Do what sriov_numvfs already does: take device_lock() so the driver
cannot be unbound underneath us, and return -ENOENT if drvdata is
NULL.

Fixes: 794b2c05ca1c ("netdevsim: extend device attrs to support port addition and deletion")
Reported-by: syzbot+b3fca27eb4005f0111f4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b3fca27eb4005f0111f4
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Reproduced in QEMU (x86_64, KASAN + lockdep) with the syzbot C repro:
the unpatched kernel oopses in nsim_drv_port_add() right after unbind.
With the patch, the write to new_port returns -ENOENT, a 40s stress of
bind/unbind + new_device/del_device racing with new_port/del_port/
sriov_numvfs runs clean (no splat, no lockdep report, no hung task),
and new_port/del_port still work as before on a bound device.
 drivers/net/netdevsim/bus.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 5c55c308487b..d843b3c985cf 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -93,8 +93,13 @@ new_port_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 	}
 
-	ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index,
-				addr_set ? eth_addr : NULL);
+	device_lock(dev);
+	ret = -ENOENT;
+	if (dev_get_drvdata(dev))
+		ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
+					port_index, addr_set ? eth_addr : NULL);
+	device_unlock(dev);
+
 	return ret ? ret : count;
 }
 
@@ -115,7 +120,13 @@ del_port_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
+	device_lock(dev);
+	ret = -ENOENT;
+	if (dev_get_drvdata(dev))
+		ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
+					port_index);
+	device_unlock(dev);
+
 	return ret ? ret : count;
 }
 
-- 
2.43.0


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

* Re: [PATCH net] netdevsim: check the driver is bound in new_port/del_port
  2026-11-01 13:58 [PATCH net] netdevsim: check the driver is bound in new_port/del_port Nguyen Ngoc Thang
@ 2026-11-02  2:19 ` Hangbin Liu
  2026-11-09  1:18 ` Hangbin Liu
  1 sibling, 0 replies; 3+ messages in thread
From: Hangbin Liu @ 2026-11-02  2:19 UTC (permalink / raw)
  To: Nguyen Ngoc Thang, Jiacheng Xu
  Cc: kuba, andrew+netdev, davem, edumazet, pabeni, netdev,
	linux-kernel, syzbot+b3fca27eb4005f0111f4

Hi Nguyen,
On Sun, Nov 01, 2026 at 08:58:24PM +0700, Nguyen Ngoc Thang wrote:
> new_port and del_port are attributes of the bus device, so they stay
> reachable after the driver is unbound. nsim_drv_remove() clears
> drvdata, yet both handlers pass it straight to priv_to_devlink() and
> devl_lock(), which faults on a small negative pointer:
> 
>   BUG: unable to handle page fault for address: fffffffffffffeb0
>   RIP: 0010:__lock_acquire+0x41/0x2de0 kernel/locking/lockdep.c:5140
>   Call Trace:
>    lock_acquire+0x115/0x350
>    mutex_lock_nested+0x55/0x1d0
>    nsim_drv_port_add+0x5a/0x140 drivers/net/netdevsim/dev.c:1760
>    new_port_store+0x1a5/0x280 drivers/net/netdevsim/bus.c:96
>    kernfs_fop_write_iter+0x3a5/0x540
> 
> The nsim_bus_dev->init check does not help: it is only cleared when
> the whole device is deleted, not on unbind.
> 
> Do what sriov_numvfs already does: take device_lock() so the driver
> cannot be unbound underneath us, and return -ENOENT if drvdata is
> NULL.
> 
> Fixes: 794b2c05ca1c ("netdevsim: extend device attrs to support port addition and deletion")
> Reported-by: syzbot+b3fca27eb4005f0111f4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b3fca27eb4005f0111f4
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

FYI, Jiacheng already posted a same fix for this issue.

https://lore.kernel.org/netdev/4fe1dcd1.16124.1a01a660a42.Coremail.stitch@zju.edu.cn/

Thanks
Hangbin

> ---
> Reproduced in QEMU (x86_64, KASAN + lockdep) with the syzbot C repro:
> the unpatched kernel oopses in nsim_drv_port_add() right after unbind.
> With the patch, the write to new_port returns -ENOENT, a 40s stress of
> bind/unbind + new_device/del_device racing with new_port/del_port/
> sriov_numvfs runs clean (no splat, no lockdep report, no hung task),
> and new_port/del_port still work as before on a bound device.
>  drivers/net/netdevsim/bus.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
> index 5c55c308487b..d843b3c985cf 100644
> --- a/drivers/net/netdevsim/bus.c
> +++ b/drivers/net/netdevsim/bus.c
> @@ -93,8 +93,13 @@ new_port_store(struct device *dev, struct device_attribute *attr,
>  		return -EINVAL;
>  	}
>  
> -	ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index,
> -				addr_set ? eth_addr : NULL);
> +	device_lock(dev);
> +	ret = -ENOENT;
> +	if (dev_get_drvdata(dev))
> +		ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
> +					port_index, addr_set ? eth_addr : NULL);
> +	device_unlock(dev);
> +
>  	return ret ? ret : count;
>  }
>  
> @@ -115,7 +120,13 @@ del_port_store(struct device *dev, struct device_attribute *attr,
>  	if (ret)
>  		return ret;
>  
> -	ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
> +	device_lock(dev);
> +	ret = -ENOENT;
> +	if (dev_get_drvdata(dev))
> +		ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
> +					port_index);
> +	device_unlock(dev);
> +
>  	return ret ? ret : count;
>  }
>  
> -- 
> 2.43.0
> 

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

* Re: [PATCH net] netdevsim: check the driver is bound in new_port/del_port
  2026-11-01 13:58 [PATCH net] netdevsim: check the driver is bound in new_port/del_port Nguyen Ngoc Thang
  2026-11-02  2:19 ` Hangbin Liu
@ 2026-11-09  1:18 ` Hangbin Liu
  1 sibling, 0 replies; 3+ messages in thread
From: Hangbin Liu @ 2026-11-09  1:18 UTC (permalink / raw)
  To: Nguyen Ngoc Thang, Jiacheng Xu, Shuai Liu, Ömer Mete Kaya
  Cc: kuba, andrew+netdev, davem, edumazet, pabeni, netdev,
	linux-kernel, syzbot+b3fca27eb4005f0111f4

On Sun, Nov 01, 2026 at 08:58:24PM +0700, Nguyen Ngoc Thang wrote:
> new_port and del_port are attributes of the bus device, so they stay
> reachable after the driver is unbound. nsim_drv_remove() clears
> drvdata, yet both handlers pass it straight to priv_to_devlink() and
> devl_lock(), which faults on a small negative pointer:
> 
>   BUG: unable to handle page fault for address: fffffffffffffeb0
>   RIP: 0010:__lock_acquire+0x41/0x2de0 kernel/locking/lockdep.c:5140
>   Call Trace:
>    lock_acquire+0x115/0x350
>    mutex_lock_nested+0x55/0x1d0
>    nsim_drv_port_add+0x5a/0x140 drivers/net/netdevsim/dev.c:1760
>    new_port_store+0x1a5/0x280 drivers/net/netdevsim/bus.c:96
>    kernfs_fop_write_iter+0x3a5/0x540
> 
> The nsim_bus_dev->init check does not help: it is only cleared when
> the whole device is deleted, not on unbind.
> 
> Do what sriov_numvfs already does: take device_lock() so the driver
> cannot be unbound underneath us, and return -ENOENT if drvdata is
> NULL.
> 
> Fixes: 794b2c05ca1c ("netdevsim: extend device attrs to support port addition and deletion")
> Reported-by: syzbot+b3fca27eb4005f0111f4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b3fca27eb4005f0111f4
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> Reproduced in QEMU (x86_64, KASAN + lockdep) with the syzbot C repro:
> the unpatched kernel oopses in nsim_drv_port_add() right after unbind.
> With the patch, the write to new_port returns -ENOENT, a 40s stress of
> bind/unbind + new_device/del_device racing with new_port/del_port/
> sriov_numvfs runs clean (no splat, no lockdep report, no hung task),
> and new_port/del_port still work as before on a bound device.
>  drivers/net/netdevsim/bus.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
> index 5c55c308487b..d843b3c985cf 100644
> --- a/drivers/net/netdevsim/bus.c
> +++ b/drivers/net/netdevsim/bus.c
> @@ -93,8 +93,13 @@ new_port_store(struct device *dev, struct device_attribute *attr,
>  		return -EINVAL;
>  	}
>  
> -	ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index,
> -				addr_set ? eth_addr : NULL);
> +	device_lock(dev);
> +	ret = -ENOENT;
> +	if (dev_get_drvdata(dev))
> +		ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
> +					port_index, addr_set ? eth_addr : NULL);
> +	device_unlock(dev);
> +
>  	return ret ? ret : count;
>  }
>  
> @@ -115,7 +120,13 @@ del_port_store(struct device *dev, struct device_attribute *attr,
>  	if (ret)
>  		return ret;
>  
> -	ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
> +	device_lock(dev);
> +	ret = -ENOENT;
> +	if (dev_get_drvdata(dev))
> +		ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
> +					port_index);
> +	device_unlock(dev);
> +
>  	return ret ? ret : count;
>  }
>  
> -- 
> 2.43.0
> 

There are three identical fixes (author cc'ed) following Jiacheng’s patch [1].
I’m not sure whether there were others before that. I notified Jiacheng and
Nguyen on Nov 2, but Jiacheng hasn’t reposted his patch. To prevent further
duplicate fixes from landing on netdev, I’d prefer to ack Nguyen’s patch,
since it is the earliest one that includes test information.

[1] https://lore.kernel.org/netdev/4fe1dcd1.16124.1a01a660a42.Coremail.stitch@zju.edu.cn/

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

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

end of thread, other threads:[~2026-11-09  1:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01 13:58 [PATCH net] netdevsim: check the driver is bound in new_port/del_port Nguyen Ngoc Thang
2026-11-02  2:19 ` Hangbin Liu
2026-11-09  1:18 ` Hangbin Liu

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®