Re: [PATCH] netdevsim: avoid NULL dereference after failed probe

[Hangbin Liu <hangbin.liu@linux.dev>]

Hi Jiacheng,

On Wed, Aug 19, 2026 at 12:42:22PM +0800, Jiacheng Xu wrote:
> device_register() reports whether device registration succeeded, not
> whether the matching driver's probe succeeded. If nsim_drv_probe()
> fails, the driver core leaves the nsim_bus_dev registered while the probe
> error path clears its driver data.
> 
> new_device_store() subsequently marks the nsim_bus_dev initialized. A
> write to its new_port or del_port attribute therefore passes the init
> check and calls nsim_drv_port_add() or nsim_drv_port_del() with no valid
> nsim_dev. Both helpers pass the NULL driver data to priv_to_devlink(),
> leading to a NULL pointer dereference. The triggering PoC is attached.
> 
> Serialize the driver data check and the port operation with the device
> lock, and reject the operation with -ENODEV when no driver data is
> present. This also prevents driver unbind from freeing nsim_dev between
> the check and its use, and follows the locking used by sriov_numvfs.
> 
> Fixes: 794b2c05ca1c ("netdevsim: extend device attrs to support port addition and deletion")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>
> ---
> 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 41483e371f05..0ab51306ad0a 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);
> +       if (!dev_get_drvdata(dev))
> +               ret = -ENODEV;
> +       else
> +               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);
> +       if (!dev_get_drvdata(dev))
> +               ret = -ENODEV;
> +       else
> +               ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF,
> +                                       port_index);
> +       device_unlock(dev);
>         return ret ? ret : count;
>  }
> 
> --
> 2.25.1

Thanks for your fixes.

Please do not attache the reproducer as an attachment in patch file. You can
describe it in commit description, cover letter, Or, add it as a selftest in
tools/testing/selftests/drivers/net/netdevsim/.

BTW, please designate your patch to a tree, i.e. [PATCH net].

Thanks
Hangbin

> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> 
> static void write_attr(const char *path, const char *buf)
> {
>         int fd = open(path, O_WRONLY | O_CLOEXEC);
>         ssize_t len;
> 
>         if (fd < 0) {
>                 perror(path);
>                 exit(EXIT_FAILURE);
>         }
>         len = write(fd, buf, strlen(buf));
>         if (len < 0)
>                 perror(path);
>         close(fd);
> }
> 
> int main(void)
> {
>         /* One port is enough; the excessive queue count makes probe fail. */
>         write_attr("/sys/bus/netdevsim/new_device",
>                    "1 1 2147483648");
>         write_attr("/sys/bus/netdevsim/devices/netdevsim1/new_port",
>                    "2");
>         return 0;
> }