Netdev List
 help / color / mirror / Atom feed
* [PATCH] netdevsim: avoid NULL dereference after failed probe
@ 2026-08-19  4:42 Jiacheng Xu
  2026-08-19  7:12 ` Hangbin Liu
  0 siblings, 1 reply; 2+ messages in thread
From: Jiacheng Xu @ 2026-08-19  4:42 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

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

[-- Attachment #2: repro.c --]
[-- Type: text/plain, Size: 744 bytes --]

#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;
}

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

* Re: [PATCH] netdevsim: avoid NULL dereference after failed probe
  2026-08-19  4:42 [PATCH] netdevsim: avoid NULL dereference after failed probe Jiacheng Xu
@ 2026-08-19  7:12 ` Hangbin Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Hangbin Liu @ 2026-08-19  7:12 UTC (permalink / raw)
  To: Jiacheng Xu
  Cc: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, netdev

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;
> }


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

end of thread, other threads:[~2026-08-19  7:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19  4:42 [PATCH] netdevsim: avoid NULL dereference after failed probe Jiacheng Xu
2026-08-19  7:12 ` Hangbin Liu

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