| Message ID | ZSkhWa5wmAGsAdCK@Sun (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next] usb/usbip: fix wrong data added to platform device | expand |
On Fri, Oct 13, 2023 at 06:52:09PM +0800, Hongren Zheng wrote: > .data of platform_device_info will be copied into .platform_data of > struct device via platform_device_add_data. > > However, vhcis[i] contains a spinlock, is dynamically allocated and > used by other code, so it is not meant to be copied. The workaround > was to use void *vhci as an agent, but it was removed in the commit > suggested below. > > This patch adds back the workaround and changes the way of using > platform_data accordingly. > > Reported-by: syzbot+e0dbc33630a092ccf033@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-usb/00000000000029242706077f3145@google.com/ > Reported-by: syzbot+6867a9777f4b8dc4e256@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-usb/0000000000007634c1060793197c@google.com/ > Fixes: b8aaf639b403 ("usbip: Use platform_device_register_full()") > Signed-off-by: Hongren Zheng <i@zenithal.me> > --- Tested-by: syzbot+6867a9777f4b8dc4e256@syzkaller.appspotmail.com Link: https://lore.kernel.org/r/0000000000007ac87d0607979b6b@google.com/
On Fri, Oct 13, 2023 at 06:52:09PM +0800, Hongren Zheng wrote: > .data of platform_device_info will be copied into .platform_data of > struct device via platform_device_add_data. > > However, vhcis[i] contains a spinlock, is dynamically allocated and > used by other code, so it is not meant to be copied. The workaround > was to use void *vhci as an agent, but it was removed in the commit > suggested below. > > This patch adds back the workaround and changes the way of using > platform_data accordingly. Thanks for fixing this. ... > - struct vhci *vhci = dev_get_platdata(hcd->self.controller); > - The blank line here is on purpose. > + struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller));
On Fri, Oct 13, 2023 at 06:52:09PM +0800, Hongren Zheng wrote: > .data of platform_device_info will be copied into .platform_data of > struct device via platform_device_add_data. > > However, vhcis[i] contains a spinlock, is dynamically allocated and > used by other code, so it is not meant to be copied. The workaround > was to use void *vhci as an agent, but it was removed in the commit > suggested below. > > This patch adds back the workaround and changes the way of using > platform_data accordingly. ... One more thing... > static int __init vhci_hcd_init(void) > { > int i, ret; > + void *vhci; > > if (usb_disabled()) > return -ENODEV; > @@ -1522,10 +1522,11 @@ static int __init vhci_hcd_init(void) > goto err_driver_register; > > for (i = 0; i < vhci_num_controllers; i++) { > + vhci = &vhcis[i]; This should be void *vhci = &vhcis[i]; because otherwise we mix code and definitions which is not so good style. (Yet we allow to do that in exceptional cases: 1) iterators in for-loops, and 2) RAII allocations with __free() in use.) > struct platform_device_info pdevinfo = { > .name = driver_name, > .id = i, > - .data = &vhcis[i], > + .data = &vhci, > .size_data = sizeof(void *), > };
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index f845b91848b9..dfbdc77108e5 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -1139,8 +1139,7 @@ static int hcd_name_to_id(const char *name) static int vhci_setup(struct usb_hcd *hcd) { - struct vhci *vhci = dev_get_platdata(hcd->self.controller); - + struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller)); if (usb_hcd_is_primary_hcd(hcd)) { vhci->vhci_hcd_hs = hcd_to_vhci_hcd(hcd); vhci->vhci_hcd_hs->vhci = vhci; @@ -1257,7 +1256,7 @@ static int vhci_get_frame_number(struct usb_hcd *hcd) /* FIXME: suspend/resume */ static int vhci_bus_suspend(struct usb_hcd *hcd) { - struct vhci *vhci = dev_get_platdata(hcd->self.controller); + struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller)); unsigned long flags; dev_dbg(&hcd->self.root_hub->dev, "%s\n", __func__); @@ -1271,7 +1270,7 @@ static int vhci_bus_suspend(struct usb_hcd *hcd) static int vhci_bus_resume(struct usb_hcd *hcd) { - struct vhci *vhci = dev_get_platdata(hcd->self.controller); + struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller)); int rc = 0; unsigned long flags; @@ -1338,7 +1337,7 @@ static const struct hc_driver vhci_hc_driver = { static int vhci_hcd_probe(struct platform_device *pdev) { - struct vhci *vhci = dev_get_platdata(&pdev->dev); + struct vhci *vhci = *((void **)dev_get_platdata(&pdev->dev)); struct usb_hcd *hcd_hs; struct usb_hcd *hcd_ss; int ret; @@ -1396,7 +1395,7 @@ static int vhci_hcd_probe(struct platform_device *pdev) static void vhci_hcd_remove(struct platform_device *pdev) { - struct vhci *vhci = dev_get_platdata(&pdev->dev); + struct vhci *vhci = *((void **)dev_get_platdata(&pdev->dev)); /* * Disconnects the root hub, @@ -1431,7 +1430,7 @@ static int vhci_hcd_suspend(struct platform_device *pdev, pm_message_t state) if (!hcd) return 0; - vhci = dev_get_platdata(hcd->self.controller); + vhci = *((void **)dev_get_platdata(hcd->self.controller)); spin_lock_irqsave(&vhci->lock, flags); @@ -1506,6 +1505,7 @@ static void del_platform_devices(void) static int __init vhci_hcd_init(void) { int i, ret; + void *vhci; if (usb_disabled()) return -ENODEV; @@ -1522,10 +1522,11 @@ static int __init vhci_hcd_init(void) goto err_driver_register; for (i = 0; i < vhci_num_controllers; i++) { + vhci = &vhcis[i]; struct platform_device_info pdevinfo = { .name = driver_name, .id = i, - .data = &vhcis[i], + .data = &vhci, .size_data = sizeof(void *), };
.data of platform_device_info will be copied into .platform_data of struct device via platform_device_add_data. However, vhcis[i] contains a spinlock, is dynamically allocated and used by other code, so it is not meant to be copied. The workaround was to use void *vhci as an agent, but it was removed in the commit suggested below. This patch adds back the workaround and changes the way of using platform_data accordingly. Reported-by: syzbot+e0dbc33630a092ccf033@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-usb/00000000000029242706077f3145@google.com/ Reported-by: syzbot+6867a9777f4b8dc4e256@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-usb/0000000000007634c1060793197c@google.com/ Fixes: b8aaf639b403 ("usbip: Use platform_device_register_full()") Signed-off-by: Hongren Zheng <i@zenithal.me> --- drivers/usb/usbip/vhci_hcd.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)