On Tue, 11 Oct 2016, Rob Herring wrote:

> The addition of CONFIG_DEBUG_TEST_DRIVER_REMOVE uncovered a bug in
> dummy_hcd driver. The problem is the gadget's struct device is allocated
> once in the initcall, so re-probing the device causes this warning:
> 
> [   13.516309] kobject (8f92f8b4): tried to init an initialized object, 
> something is seriously wrong.
> [   13.518358] CPU: 0 PID: 1 Comm: swapper Not tainted 
> 4.8.0-rc4-00003-gbea5b15 #1
> [   13.520014] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> Debian-1.8.2-1 04/01/2014
> [   13.522207]  8f92f8b4 8f92f8b4 80031d20 88bb56c9 80031d3c 88bb7dd3 
> 898316d4 8f92f8b4
> [   13.524132]  8f92f868 899b8c6c 8f92f8ac 80031d4c 88d83cce 8f92f868 
> 8f92f8ac 80031d58
> [   13.526087]  88d844bb 8f92f868 80031d84 88ef3c1d 00000000 89863eac 
> 00000000 8004f760
> [   13.527980] Call Trace:
> [   13.528540]  [<88bb56c9>] dump_stack+0x16/0x1d
> [   13.529544]  [<88bb7dd3>] kobject_init+0x73/0x80
> [   13.530602]  [<88d83cce>] device_initialize+0x1e/0xe0
> [   13.531889]  [<88d844bb>] device_register+0xb/0x20
> [   13.532997]  [<88ef3c1d>] usb_add_gadget_udc_release+0x8d/0x270
> [   13.534333]  [<88ef3e9a>] usb_add_gadget_udc+0xa/0x10
> [   13.535465]  [<88ef775e>] dummy_udc_probe+0x14e/0x1a0
> [   13.536623]  [<88d89781>] platform_drv_probe+0x31/0x90
> [   13.537830]  [<88d875aa>] ? driver_sysfs_add+0x6a/0x90
> [   13.539014]  [<88d87e3a>] driver_probe_device+0x12a/0x490
> [   13.540228]  [<88cbc39b>] ? acpi_driver_match_device+0x36/0x50
> [   13.541658]  [<88d88307>] __device_attach_driver+0x77/0x110
> [   13.542946]  [<8949712d>] ? klist_next+0x6d/0x10c
> [   13.544053]  [<88d88290>] ? __driver_attach+0xf0/0xf0
> [   13.545180]  [<88d864f7>] bus_for_each_drv+0x47/0x80
> [   13.549626]  [<88d87b85>] __device_attach+0xb5/0x130
> [   13.550493]  [<88d88290>] ? __driver_attach+0xf0/0xf0
> [   13.551517]  [<88d883cd>] device_initial_probe+0xd/0x10
> [   13.552485]  [<88d86787>] bus_probe_device+0x77/0x80
> [   13.553298]  [<88d8417e>] device_add+0x34e/0x5a0
> [   13.554025]  [<88bc4840>] ? kvasprintf_const+0x40/0x90
> [   13.554860]  [<88bb7d1b>] ? kobject_set_name_vargs+0x6b/0x90
> [   13.555770]  [<88d89e6c>] platform_device_add+0xfc/0x280
> [   13.556640]  [<89ad0b84>] init+0x20b/0x2ec
> [   13.557317]  [<89ad0979>] ? usb_udc_init+0x3f/0x3f
> [   13.558154]  [<89a96c1d>] do_one_initcall+0x7c/0xfb
> [   13.558947]  [<89a96d5e>] ? kernel_init_freeable+0xc2/0x15e
> [   13.559851]  [<89a96d81>] kernel_init_freeable+0xe5/0x15e
> [   13.560726]  [<894974fb>] kernel_init+0xb/0x100
> [   13.561705]  [<888c727c>] ? schedule_tail+0xc/0x50
> [   13.562786]  [<894a1942>] ret_from_kernel_thread+0xe/0x24
> [   13.563993]  [<894974f0>] ? rest_init+0x110/0x110
> 
> Fix this by clearing struct gadget containing the struct device on
> removal. This could also be fixed by disabling bind/unbind, but that is
> perhaps a useful feature to simulate device attach/detach.
> 
> Reported-by: kernel test robot <xiaolong...@intel.com>
> Cc: Felipe Balbi <ba...@kernel.org>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Rob Herring <r...@kernel.org>
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c 
> b/drivers/usb/gadget/udc/dummy_hcd.c
> index 77d07904f932..64f3d3cdcb91 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1060,6 +1060,7 @@ static int dummy_udc_remove(struct platform_device 
> *pdev)
>  
>       device_remove_file(&dum->gadget.dev, &dev_attr_function);
>       usb_del_gadget_udc(&dum->gadget);
> +     memset(&dum->gadget, 0, sizeof(dum->gadget));
>       return 0;
>  }

Have you checked that no other parts of the driver are still using the
data structure while it is zeroed?  What about the host-side parts?  I 
see a few calls to udc_dev() in host code dev_dbg() lines, and 
references to fields in dum->gadget in more active parts of the code.

It might be a lot simpler in the end to forbid rebinding.  And if 
people want to support emulated attach/detach, it can be done via a 
writable sysfs attribute.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to