[PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[Deepanshu Kartikey]

The vhci attribute group is created in vhci_start() and removed in
vhci_stop(), guarded by usb_hcd_is_primary_hcd(). Both run from
usb_add_hcd()/usb_remove_hcd(), which are called once per hcd, so the
attach attribute is live while only one of the two hcds exists: it is
created during the first usb_add_hcd() before vhci_hcd_ss is set, and it
survives the first usb_put_hcd() on removal. A concurrent write to attach
can therefore reach a NULL or freed vhci_hcd_ss.

Register the group as dev_groups on the platform driver instead, so the
driver core creates the files before probe and removes them after remove
returns, and drop the sysfs_create_group()/sysfs_remove_group() calls from
the driver. The attributes have always been created only on vhci_hcd.0;
an is_visible() callback keeps them there.

vhci_init_attr_group() is moved into vhci_hcd_init() ahead of
platform_driver_register() so the group is populated before the core
reads it. The attribute array is still built at runtime because the
number of status attributes comes from CONFIG_USBIP_VHCI_NR_HCS and the
preprocessor cannot emit a variable number of __ATTR() declarations.

Fixes: 1c9de5bf4286 ("usbip: vhci-hcd: Add USB3 SuperSpeed support")
Reported-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8753715f05759f1a10de
Tested-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/20260815143427.19066-1-kartikey406@gmail.com/T/ [v1]
Link: https://lore.kernel.org/all/20260816015051.13184-1-kartikey406@gmail.com/T/ [v2]
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v3:
 - make vhci_attr_group static, drop its extern from vhci.h (Greg)
 - drop stray blank line in vhci_hcd_init()
 - fix indentation in vhci_sysfs.c

v2:
 - use dev_groups instead of moving sysfs_create_group() into probe (Greg)
 - add is_visible() to keep the attributes on vhci_hcd.0 only
 - move vhci_init_attr_group() into vhci_hcd_init()

 drivers/usb/usbip/vhci.h       |  2 +-
 drivers/usb/usbip/vhci_hcd.c   | 36 ++++++++++------------------------
 drivers/usb/usbip/vhci_sysfs.c | 21 +++++++++++++++++++-
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
index 5659dce1526e..f3993238f91d 100644
--- a/drivers/usb/usbip/vhci.h
+++ b/drivers/usb/usbip/vhci.h
@@ -120,7 +120,7 @@ struct vhci_hcd {
 
 extern int vhci_num_controllers;
 extern struct vhci *vhcis;
-extern struct attribute_group vhci_attr_group;
+extern const struct attribute_group *vhci_groups[];
 
 /* vhci_hcd.c */
 void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed);
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 39e8faf4c18c..22b9bad7ca31 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1199,7 +1199,6 @@ static int vhci_start(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
 	int id, rhport;
-	int err;
 
 	usbip_dbg_vhci_hc("enter vhci_start\n");
 
@@ -1230,40 +1229,17 @@ static int vhci_start(struct usb_hcd *hcd)
 		return -EINVAL;
 	}
 
-	/* vhci_hcd is now ready to be controlled through sysfs */
-	if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
-		err = vhci_init_attr_group();
-		if (err) {
-			dev_err(hcd_dev(hcd), "init attr group failed, err = %d\n", err);
-			return err;
-		}
-		err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
-		if (err) {
-			dev_err(hcd_dev(hcd), "create sysfs files failed, err = %d\n", err);
-			vhci_finish_attr_group();
-			return err;
-		}
-		dev_info(hcd_dev(hcd), "created sysfs %s\n", hcd_name(hcd));
-	}
-
 	return 0;
 }
 
 static void vhci_stop(struct usb_hcd *hcd)
 {
 	struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
-	int id, rhport;
+	int rhport;
 
 	usbip_dbg_vhci_hc("stop VHCI controller\n");
 
-	/* 1. remove the userland interface of vhci_hcd */
-	id = hcd_name_to_id(hcd_name(hcd));
-	if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
-		sysfs_remove_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
-		vhci_finish_attr_group();
-	}
-
-	/* 2. shutdown all the ports of vhci_hcd */
+	/* shutdown all the ports of vhci_hcd */
 	for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
 		struct vhci_device *vdev = &vhci_hcd->vdev[rhport];
 
@@ -1513,6 +1489,7 @@ static struct platform_driver vhci_driver = {
 	.resume	= vhci_hcd_resume,
 	.driver	= {
 		.name = driver_name,
+		.dev_groups = vhci_groups,
 	},
 };
 
@@ -1541,6 +1518,10 @@ static int __init vhci_hcd_init(void)
 	if (vhcis == NULL)
 		return -ENOMEM;
 
+	ret = vhci_init_attr_group();
+	if (ret)
+		goto err_init_attr_group;
+
 	ret = platform_driver_register(&vhci_driver);
 	if (ret)
 		goto err_driver_register;
@@ -1568,6 +1549,8 @@ static int __init vhci_hcd_init(void)
 err_add_hcd:
 	platform_driver_unregister(&vhci_driver);
 err_driver_register:
+	vhci_finish_attr_group();
+err_init_attr_group:
 	kfree(vhcis);
 	return ret;
 }
@@ -1576,6 +1559,7 @@ static void __exit vhci_hcd_exit(void)
 {
 	del_platform_devices();
 	platform_driver_unregister(&vhci_driver);
+	vhci_finish_attr_group();
 	kfree(vhcis);
 }
 
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index a7ede6fb3da9..d87d9e449d72 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -497,8 +497,27 @@ static void finish_status_attrs(void)
 	kfree(status_attrs);
 }
 
-struct attribute_group vhci_attr_group = {
+static umode_t vhci_attr_is_visible(struct kobject *kobj,
+				    struct attribute *attr, int n)
+{
+	struct platform_device *pdev = to_platform_device(kobj_to_dev(kobj));
+
+	/*
+	 * The attributes control every controller and have always lived on
+	 * vhci_hcd.0 only.  Keep them there now that the driver core creates
+	 * the group for each device.
+	 */
+	return pdev->id == 0 ? attr->mode : 0;
+}
+
+static struct attribute_group vhci_attr_group = {
 	.attrs = NULL,
+	.is_visible = vhci_attr_is_visible,
+};
+
+const struct attribute_group *vhci_groups[] = {
+	&vhci_attr_group,
+	NULL,
 };
 
 int vhci_init_attr_group(void)
-- 
2.43.0

Re: [PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[Deepanshu Kartikey]

On Sun, Aug 16, 2026 at 11:40 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> The vhci attribute group is created in vhci_start() and removed in
> vhci_stop(), guarded by usb_hcd_is_primary_hcd(). Both run from
> usb_add_hcd()/usb_remove_hcd(), which are called once per hcd, so the
> attach attribute is live while only one of the two hcds exists: it is
> created during the first usb_add_hcd() before vhci_hcd_ss is set, and it
> survives the first usb_put_hcd() on removal. A concurrent write to attach
> can therefore reach a NULL or freed vhci_hcd_ss.
>
> Register the group as dev_groups on the platform driver instead, so the
> driver core creates the files before probe and removes them after remove
> returns, and drop the sysfs_create_group()/sysfs_remove_group() calls from
> the driver. The attributes have always been created only on vhci_hcd.0;
> an is_visible() callback keeps them there.
>
> vhci_init_attr_group() is moved into vhci_hcd_init() ahead of
> platform_driver_register() so the group is populated before the core
> reads it. The attribute array is still built at runtime because the
> number of status attributes comes from CONFIG_USBIP_VHCI_NR_HCS and the
> preprocessor cannot emit a variable number of __ATTR() declarations.
>
> Fixes: 1c9de5bf4286 ("usbip: vhci-hcd: Add USB3 SuperSpeed support")
> Reported-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8753715f05759f1a10de
> Tested-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/all/20260815143427.19066-1-kartikey406@gmail.com/T/ [v1]
> Link: https://lore.kernel.org/all/20260816015051.13184-1-kartikey406@gmail.com/T/ [v2]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> v3:
>  - make vhci_attr_group static, drop its extern from vhci.h (Greg)
>  - drop stray blank line in vhci_hcd_init()
>  - fix indentation in vhci_sysfs.c
>
> v2:
>  - use dev_groups instead of moving sysfs_create_group() into probe (Greg)
>  - add is_visible() to keep the attributes on vhci_hcd.0 only
>  - move vhci_init_attr_group() into vhci_hcd_init()
>
>  drivers/usb/usbip/vhci.h       |  2 +-
>  drivers/usb/usbip/vhci_hcd.c   | 36 ++++++++++------------------------
>  drivers/usb/usbip/vhci_sysfs.c | 21 +++++++++++++++++++-
>  3 files changed, 31 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/usb/usbip/vhci.h b/drivers/usb/usbip/vhci.h
> index 5659dce1526e..f3993238f91d 100644
> --- a/drivers/usb/usbip/vhci.h
> +++ b/drivers/usb/usbip/vhci.h
> @@ -120,7 +120,7 @@ struct vhci_hcd {
>
>  extern int vhci_num_controllers;
>  extern struct vhci *vhcis;
> -extern struct attribute_group vhci_attr_group;
> +extern const struct attribute_group *vhci_groups[];
>
>  /* vhci_hcd.c */
>  void rh_port_connect(struct vhci_device *vdev, enum usb_device_speed speed);
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 39e8faf4c18c..22b9bad7ca31 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1199,7 +1199,6 @@ static int vhci_start(struct usb_hcd *hcd)
>  {
>         struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
>         int id, rhport;
> -       int err;
>
>         usbip_dbg_vhci_hc("enter vhci_start\n");
>
> @@ -1230,40 +1229,17 @@ static int vhci_start(struct usb_hcd *hcd)
>                 return -EINVAL;
>         }
>
> -       /* vhci_hcd is now ready to be controlled through sysfs */
> -       if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
> -               err = vhci_init_attr_group();
> -               if (err) {
> -                       dev_err(hcd_dev(hcd), "init attr group failed, err = %d\n", err);
> -                       return err;
> -               }
> -               err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
> -               if (err) {
> -                       dev_err(hcd_dev(hcd), "create sysfs files failed, err = %d\n", err);
> -                       vhci_finish_attr_group();
> -                       return err;
> -               }
> -               dev_info(hcd_dev(hcd), "created sysfs %s\n", hcd_name(hcd));
> -       }
> -
>         return 0;
>  }
>
>  static void vhci_stop(struct usb_hcd *hcd)
>  {
>         struct vhci_hcd *vhci_hcd = hcd_to_vhci_hcd(hcd);
> -       int id, rhport;
> +       int rhport;
>
>         usbip_dbg_vhci_hc("stop VHCI controller\n");
>
> -       /* 1. remove the userland interface of vhci_hcd */
> -       id = hcd_name_to_id(hcd_name(hcd));
> -       if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
> -               sysfs_remove_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
> -               vhci_finish_attr_group();
> -       }
> -
> -       /* 2. shutdown all the ports of vhci_hcd */
> +       /* shutdown all the ports of vhci_hcd */
>         for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
>                 struct vhci_device *vdev = &vhci_hcd->vdev[rhport];
>
> @@ -1513,6 +1489,7 @@ static struct platform_driver vhci_driver = {
>         .resume = vhci_hcd_resume,
>         .driver = {
>                 .name = driver_name,
> +               .dev_groups = vhci_groups,
>         },
>  };
>
> @@ -1541,6 +1518,10 @@ static int __init vhci_hcd_init(void)
>         if (vhcis == NULL)
>                 return -ENOMEM;
>
> +       ret = vhci_init_attr_group();
> +       if (ret)
> +               goto err_init_attr_group;
> +
>         ret = platform_driver_register(&vhci_driver);
>         if (ret)
>                 goto err_driver_register;
> @@ -1568,6 +1549,8 @@ static int __init vhci_hcd_init(void)
>  err_add_hcd:
>         platform_driver_unregister(&vhci_driver);
>  err_driver_register:
> +       vhci_finish_attr_group();
> +err_init_attr_group:
>         kfree(vhcis);
>         return ret;
>  }
> @@ -1576,6 +1559,7 @@ static void __exit vhci_hcd_exit(void)
>  {
>         del_platform_devices();
>         platform_driver_unregister(&vhci_driver);
> +       vhci_finish_attr_group();
>         kfree(vhcis);
>  }
>
> diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
> index a7ede6fb3da9..d87d9e449d72 100644
> --- a/drivers/usb/usbip/vhci_sysfs.c
> +++ b/drivers/usb/usbip/vhci_sysfs.c
> @@ -497,8 +497,27 @@ static void finish_status_attrs(void)
>         kfree(status_attrs);
>  }
>
> -struct attribute_group vhci_attr_group = {
> +static umode_t vhci_attr_is_visible(struct kobject *kobj,
> +                                   struct attribute *attr, int n)
> +{
> +       struct platform_device *pdev = to_platform_device(kobj_to_dev(kobj));
> +
> +       /*
> +        * The attributes control every controller and have always lived on
> +        * vhci_hcd.0 only.  Keep them there now that the driver core creates
> +        * the group for each device.
> +        */
> +       return pdev->id == 0 ? attr->mode : 0;
> +}
> +
> +static struct attribute_group vhci_attr_group = {
>         .attrs = NULL,
> +       .is_visible = vhci_attr_is_visible,
> +};
> +
> +const struct attribute_group *vhci_groups[] = {
> +       &vhci_attr_group,
> +       NULL,
>  };
>
>  int vhci_init_attr_group(void)
> --
> 2.43.0
>

Hi Greg,

Please let me know the status of this patch

Thanks

Deepanshu

Re: [PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[Greg KH]

On Sun, Aug 30, 2026 at 11:53:30AM +0530, Deepanshu Kartikey wrote:
> Please let me know the status of this patch

It has been the merge window, where no new patches can be accepted.
When that is over we will get to it starting next week.

To make it move quicker, please help review other patches on the mailing
list to reduce our load.

thanks,

greg k-h

Re: [PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[Greg KH]

On Sun, Aug 16, 2026 at 11:40:34AM +0530, Deepanshu Kartikey wrote:
> The vhci attribute group is created in vhci_start() and removed in
> vhci_stop(), guarded by usb_hcd_is_primary_hcd(). Both run from
> usb_add_hcd()/usb_remove_hcd(), which are called once per hcd, so the
> attach attribute is live while only one of the two hcds exists: it is
> created during the first usb_add_hcd() before vhci_hcd_ss is set, and it
> survives the first usb_put_hcd() on removal. A concurrent write to attach
> can therefore reach a NULL or freed vhci_hcd_ss.
> 
> Register the group as dev_groups on the platform driver instead, so the
> driver core creates the files before probe and removes them after remove
> returns, and drop the sysfs_create_group()/sysfs_remove_group() calls from
> the driver. The attributes have always been created only on vhci_hcd.0;
> an is_visible() callback keeps them there.
> 
> vhci_init_attr_group() is moved into vhci_hcd_init() ahead of
> platform_driver_register() so the group is populated before the core
> reads it. The attribute array is still built at runtime because the
> number of status attributes comes from CONFIG_USBIP_VHCI_NR_HCS and the
> preprocessor cannot emit a variable number of __ATTR() declarations.
> 
> Fixes: 1c9de5bf4286 ("usbip: vhci-hcd: Add USB3 SuperSpeed support")
> Reported-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8753715f05759f1a10de
> Tested-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/all/20260815143427.19066-1-kartikey406@gmail.com/T/ [v1]
> Link: https://lore.kernel.org/all/20260816015051.13184-1-kartikey406@gmail.com/T/ [v2]
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---

Did you forget an Assisted-by: tag?

Re: [PATCH v3] usbip: vhci_hcd: let the driver core manage the sysfs attributes

[Deepanshu Kartikey]

On Mon, Aug 31, 2026 at 3:36 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Aug 16, 2026 at 11:40:34AM +0530, Deepanshu Kartikey wrote:
> > The vhci attribute group is created in vhci_start() and removed in
> > vhci_stop(), guarded by usb_hcd_is_primary_hcd(). Both run from
> > usb_add_hcd()/usb_remove_hcd(), which are called once per hcd, so the
> > attach attribute is live while only one of the two hcds exists: it is
> > created during the first usb_add_hcd() before vhci_hcd_ss is set, and it
> > survives the first usb_put_hcd() on removal. A concurrent write to attach
> > can therefore reach a NULL or freed vhci_hcd_ss.
> >
> > Register the group as dev_groups on the platform driver instead, so the
> > driver core creates the files before probe and removes them after remove
> > returns, and drop the sysfs_create_group()/sysfs_remove_group() calls from
> > the driver. The attributes have always been created only on vhci_hcd.0;
> > an is_visible() callback keeps them there.
> >
> > vhci_init_attr_group() is moved into vhci_hcd_init() ahead of
> > platform_driver_register() so the group is populated before the core
> > reads it. The attribute array is still built at runtime because the
> > number of status attributes comes from CONFIG_USBIP_VHCI_NR_HCS and the
> > preprocessor cannot emit a variable number of __ATTR() declarations.
> >
> > Fixes: 1c9de5bf4286 ("usbip: vhci-hcd: Add USB3 SuperSpeed support")
> > Reported-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=8753715f05759f1a10de
> > Tested-by: syzbot+8753715f05759f1a10de@syzkaller.appspotmail.com
> > Link: https://lore.kernel.org/all/20260815143427.19066-1-kartikey406@gmail.com/T/ [v1]
> > Link: https://lore.kernel.org/all/20260816015051.13184-1-kartikey406@gmail.com/T/ [v2]
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
>
> Did you forget an Assisted-by: tag?
>

Thanks for reminding me. I will add it and send patch v4.

Deepanshu