[PATCH] usb: gadget: avoid using gadget after freed

[PATCH] usb: gadget: avoid using gadget after freed

[Lianwei Wang]

The udc and gadget device will be deleted when udc device is
disconnected and the related function will be unbind with it.

But if the configfs is not deleted, then the function object
will be kept and the bound status is kept true.

Then after udc device is connected again and a new udc and
gadget objects will be created and passed to bind interface.
But because the bound is still true, the new gadget is not
updated to netdev and a previous freed gadget will be used
in netdev after bind.

To fix this using after freed issue, always set the gadget
object to netdev in bind interface.

Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
---
 drivers/usb/gadget/function/f_ecm.c    | 10 ++++++----
 drivers/usb/gadget/function/f_eem.c    | 10 ++++++----
 drivers/usb/gadget/function/f_ncm.c    | 11 +++++++----
 drivers/usb/gadget/function/f_phonet.c |  2 +-
 drivers/usb/gadget/function/f_rndis.c  |  2 +-
 drivers/usb/gadget/function/f_subset.c | 10 ++++++----
 6 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 6ce044008cf6..ff39b7eafec7 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -695,15 +695,17 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to ecm_opts->bound access
 	 */
+	mutex_lock(&ecm_opts->lock);
+	gether_set_gadget(ecm_opts->net, cdev->gadget);
 	if (!ecm_opts->bound) {
-		mutex_lock(&ecm_opts->lock);
-		gether_set_gadget(ecm_opts->net, cdev->gadget);
 		status = gether_register_netdev(ecm_opts->net);
-		mutex_unlock(&ecm_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&ecm_opts->lock);
 			return status;
+		}
 		ecm_opts->bound = true;
 	}
+	mutex_unlock(&ecm_opts->lock);
 
 	ecm_string_defs[1].s = ecm->ethaddr;
 
diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
index c13befa31110..589862dfe1e7 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -256,15 +256,17 @@ static int eem_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to eem_opts->bound access
 	 */
+	mutex_lock(&eem_opts->lock);
+	gether_set_gadget(eem_opts->net, cdev->gadget);
 	if (!eem_opts->bound) {
-		mutex_lock(&eem_opts->lock);
-		gether_set_gadget(eem_opts->net, cdev->gadget);
 		status = gether_register_netdev(eem_opts->net);
-		mutex_unlock(&eem_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&eem_opts->lock);
 			return status;
+		}
 		eem_opts->bound = true;
 	}
+	mutex_unlock(&eem_opts->lock);
 
 	us = usb_gstrings_attach(cdev, eem_strings,
 				 ARRAY_SIZE(eem_string_defs));
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 2d6e76e4cffa..951867e230c2 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1409,15 +1409,18 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to ncm_opts->bound access
 	 */
+	mutex_lock(&ncm_opts->lock);
+	gether_set_gadget(ncm_opts->net, cdev->gadget);
 	if (!ncm_opts->bound) {
-		mutex_lock(&ncm_opts->lock);
-		gether_set_gadget(ncm_opts->net, cdev->gadget);
 		status = gether_register_netdev(ncm_opts->net);
-		mutex_unlock(&ncm_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&ncm_opts->lock);
 			goto fail;
+		}
 		ncm_opts->bound = true;
 	}
+	mutex_unlock(&ncm_opts->lock);
+
 	us = usb_gstrings_attach(cdev, ncm_strings,
 				 ARRAY_SIZE(ncm_string_defs));
 	if (IS_ERR(us)) {
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index 8b72b192c747..e93c5cf95494 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -494,8 +494,8 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to phonet_opts->bound access
 	 */
+	gphonet_set_gadget(phonet_opts->net, gadget);
 	if (!phonet_opts->bound) {
-		gphonet_set_gadget(phonet_opts->net, gadget);
 		status = gphonet_register_netdev(phonet_opts->net);
 		if (status)
 			return status;
diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
index d48df36622b7..f7e59b482d55 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -698,8 +698,8 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to rndis_opts->bound access
 	 */
+	gether_set_gadget(rndis_opts->net, cdev->gadget);
 	if (!rndis_opts->bound) {
-		gether_set_gadget(rndis_opts->net, cdev->gadget);
 		status = gether_register_netdev(rndis_opts->net);
 		if (status)
 			goto fail;
diff --git a/drivers/usb/gadget/function/f_subset.c b/drivers/usb/gadget/function/f_subset.c
index 4d945254905d..878c0eb9efbd 100644
--- a/drivers/usb/gadget/function/f_subset.c
+++ b/drivers/usb/gadget/function/f_subset.c
@@ -308,15 +308,17 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
 	 * with list_for_each_entry, so we assume no race condition
 	 * with regard to gether_opts->bound access
 	 */
+	mutex_lock(&gether_opts->lock);
+	gether_set_gadget(gether_opts->net, cdev->gadget);
 	if (!gether_opts->bound) {
-		mutex_lock(&gether_opts->lock);
-		gether_set_gadget(gether_opts->net, cdev->gadget);
 		status = gether_register_netdev(gether_opts->net);
-		mutex_unlock(&gether_opts->lock);
-		if (status)
+		if (status) {
+			mutex_unlock(&gether_opts->lock);
 			return status;
+		}
 		gether_opts->bound = true;
 	}
+	mutex_unlock(&gether_opts->lock);
 
 	us = usb_gstrings_attach(cdev, geth_strings,
 				 ARRAY_SIZE(geth_string_defs));
-- 
2.17.1

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Felipe Balbi]

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

Lianwei Wang <lianwei.wang@gmail.com> writes:

> The udc and gadget device will be deleted when udc device is
> disconnected and the related function will be unbind with it.
>
> But if the configfs is not deleted, then the function object
> will be kept and the bound status is kept true.
>
> Then after udc device is connected again and a new udc and
> gadget objects will be created and passed to bind interface.
> But because the bound is still true, the new gadget is not
> updated to netdev and a previous freed gadget will be used
> in netdev after bind.
>
> To fix this using after freed issue, always set the gadget
> object to netdev in bind interface.
>
> Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>

I can't actually understand what's the problem here. The gadget is not
deleted when we disconnect the cable.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Lianwei Wang]

On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
>
> Lianwei Wang <lianwei.wang@gmail.com> writes:
>
> > The udc and gadget device will be deleted when udc device is
> > disconnected and the related function will be unbind with it.
> >
> > But if the configfs is not deleted, then the function object
> > will be kept and the bound status is kept true.
> >
> > Then after udc device is connected again and a new udc and
> > gadget objects will be created and passed to bind interface.
> > But because the bound is still true, the new gadget is not
> > updated to netdev and a previous freed gadget will be used
> > in netdev after bind.
> >
> > To fix this using after freed issue, always set the gadget
> > object to netdev in bind interface.
> >
> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
>
> I can't actually understand what's the problem here. The gadget is not
> deleted when we disconnect the cable.
>
> --
> balbi

The issue was observed with a dual-role capable USB controller (e.g. Intel
XHCI controller), which has the ability to switch role between host and device
mode. The gadget is deleted when we switch role to device mode from host
mode. See below log:
# echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
[   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
[   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
[   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
[   41.191192] usb 1-1: USB disconnect, device number 3
[   41.191284] usb 1-1.1: USB disconnect, device number 4
[   41.218958] usb 1-1.5: USB disconnect, device number 5
[   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
[   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
[   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
[   41.263413] configfs-gadget gadget: unbind function 'Function FS
Gadget'/ffff8801db049e38
[   41.263969] configfs-gadget gadget: unbind function
'cdc_network'/ffff8801d8897400
[   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
[   41.720957] dabr_udc deleted
[   41.721097] dabridge 1-5 deleted

The UDC and gadget will be deleted after switch role to device mode.
And they will be
created as new object when switching back to host mode. At this time
the bind in function
driver (e.g. f_ncm) will not set the new gadget.

For kernel 4.19+, the role switch command will be:
  echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role

The latest Intel role switch kernel driver can be found here:
  https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c

Thanks,
Lianwei

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Felipe Balbi]

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


Hi,

Lianwei Wang <lianwei.wang@gmail.com> writes:
> On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
>>
>> Lianwei Wang <lianwei.wang@gmail.com> writes:
>>
>> > The udc and gadget device will be deleted when udc device is
>> > disconnected and the related function will be unbind with it.
>> >
>> > But if the configfs is not deleted, then the function object
>> > will be kept and the bound status is kept true.
>> >
>> > Then after udc device is connected again and a new udc and
>> > gadget objects will be created and passed to bind interface.
>> > But because the bound is still true, the new gadget is not
>> > updated to netdev and a previous freed gadget will be used
>> > in netdev after bind.
>> >
>> > To fix this using after freed issue, always set the gadget
>> > object to netdev in bind interface.
>> >
>> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
>>
>> I can't actually understand what's the problem here. The gadget is not
>> deleted when we disconnect the cable.
>>
>> --
>> balbi
>
> The issue was observed with a dual-role capable USB controller (e.g. Intel
> XHCI controller), which has the ability to switch role between host and device
> mode. The gadget is deleted when we switch role to device mode from host
> mode. See below log:
> # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)

oh, so you're using a modified tree :-) Then we can't really help.

> [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
> [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
> [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
> [   41.191192] usb 1-1: USB disconnect, device number 3
> [   41.191284] usb 1-1.1: USB disconnect, device number 4
> [   41.218958] usb 1-1.5: USB disconnect, device number 5
> [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
> [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED

What is this android_work. That doesn't exist upstream.

> [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
> [   41.263413] configfs-gadget gadget: unbind function 'Function FS
> Gadget'/ffff8801db049e38
> [   41.263969] configfs-gadget gadget: unbind function
> 'cdc_network'/ffff8801d8897400
> [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
> [   41.720957] dabr_udc deleted
> [   41.721097] dabridge 1-5 deleted
>
> The UDC and gadget will be deleted after switch role to device mode.
> And they will be
> created as new object when switching back to host mode. At this time
> the bind in function
> driver (e.g. f_ncm) will not set the new gadget.
>
> For kernel 4.19+, the role switch command will be:
>   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
>
> The latest Intel role switch kernel driver can be found here:
>   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c

Right, please test against v5.2-rc5 and show me the problem on that
kernel. I can't apply patches for problems that may not even exist in
upstream, sorry.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Lianwei Wang]

On Tue, Jun 18, 2019 at 12:21 AM Felipe Balbi <balbi@kernel.org> wrote:
>
>
> Hi,
>
> Lianwei Wang <lianwei.wang@gmail.com> writes:
> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
> >>
> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >>
> >> > The udc and gadget device will be deleted when udc device is
> >> > disconnected and the related function will be unbind with it.
> >> >
> >> > But if the configfs is not deleted, then the function object
> >> > will be kept and the bound status is kept true.
> >> >
> >> > Then after udc device is connected again and a new udc and
> >> > gadget objects will be created and passed to bind interface.
> >> > But because the bound is still true, the new gadget is not
> >> > updated to netdev and a previous freed gadget will be used
> >> > in netdev after bind.
> >> >
> >> > To fix this using after freed issue, always set the gadget
> >> > object to netdev in bind interface.
> >> >
> >> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
> >>
> >> I can't actually understand what's the problem here. The gadget is not
> >> deleted when we disconnect the cable.
> >>
> >> --
> >> balbi
> >
> > The issue was observed with a dual-role capable USB controller (e.g. Intel
> > XHCI controller), which has the ability to switch role between host and device
> > mode. The gadget is deleted when we switch role to device mode from host
> > mode. See below log:
> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
>
> oh, so you're using a modified tree :-) Then we can't really help.
>
> > [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
> > [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
> > [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
> > [   41.191192] usb 1-1: USB disconnect, device number 3
> > [   41.191284] usb 1-1.1: USB disconnect, device number 4
> > [   41.218958] usb 1-1.5: USB disconnect, device number 5
> > [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
> > [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
>
> What is this android_work. That doesn't exist upstream.
>
> > [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
> > [   41.263413] configfs-gadget gadget: unbind function 'Function FS
> > Gadget'/ffff8801db049e38
> > [   41.263969] configfs-gadget gadget: unbind function
> > 'cdc_network'/ffff8801d8897400
> > [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
> > [   41.720957] dabr_udc deleted
> > [   41.721097] dabridge 1-5 deleted
> >
> > The UDC and gadget will be deleted after switch role to device mode.
> > And they will be
> > created as new object when switching back to host mode. At this time
> > the bind in function
> > driver (e.g. f_ncm) will not set the new gadget.
> >
> > For kernel 4.19+, the role switch command will be:
> >   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
> >
> > The latest Intel role switch kernel driver can be found here:
> >   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c
>
> Right, please test against v5.2-rc5 and show me the problem on that
> kernel. I can't apply patches for problems that may not even exist in
> upstream, sorry.
>
> --
> balbi

The issue exist in main line kernel, but I can not test it with
v5.2-rc5 kernel. I tested it with 4.19 kernel,
which, for the usb gadget part, has almost the same code as v5.2. It
is 100% reproducible with dual role
USB controller or by removing UDC hardware. Take f_ncm for example,
the use case as follows:

1. USB controller is in host mode, f_ncm and UDC is configured in configfs.
   - The ncm is instanced and alloced when "functions/ncm.usb0" is
created and it will be freed
      when those files are delted from configfs.

2. enable the gadget and bind it to this ncm function.
    - For the first time running, ncm_opts->bound is 0 and
gether_set_gadget is called to set the
      gadget. The bound is set to 1 then.

3. If the UDC is disconnected from bus, then the UDC and its gadget is
deleted. But because the
    ncm.usb0 is still there, ncm object is not freed and
ncm_opts->bound is still set.
    There are two ways to disconnect the UDC hardware. One is for dual
role host controller by switch
    host controller role to device mode. Another way is by removing
the UDC hardware from bus, both
    will generate an usb device disconnect event to UDC driver to
delete udc and gadget.

4. Now the bound of ncm is still set and gadget is deleted due to udc
disconnected. And if we connect
    the udc device again, then it will create new udc and gadget and
bind to ncm again. But because
    bound is already set, the new gadget is not set to gether
(gether_register_netdev not called).

Not sure if this is clear to you. Please review the scenario and the patch.

Thanks,
Lianwei

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Felipe Balbi]

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


Hi,

Lianwei Wang <lianwei.wang@gmail.com> writes:
>> Lianwei Wang <lianwei.wang@gmail.com> writes:
>> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
>> >>
>> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
>> >>
>> >> > The udc and gadget device will be deleted when udc device is
>> >> > disconnected and the related function will be unbind with it.
>> >> >
>> >> > But if the configfs is not deleted, then the function object
>> >> > will be kept and the bound status is kept true.
>> >> >
>> >> > Then after udc device is connected again and a new udc and
>> >> > gadget objects will be created and passed to bind interface.
>> >> > But because the bound is still true, the new gadget is not
>> >> > updated to netdev and a previous freed gadget will be used
>> >> > in netdev after bind.
>> >> >
>> >> > To fix this using after freed issue, always set the gadget
>> >> > object to netdev in bind interface.
>> >> >
>> >> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
>> >>
>> >> I can't actually understand what's the problem here. The gadget is not
>> >> deleted when we disconnect the cable.
>> >>
>> >> --
>> >> balbi
>> >
>> > The issue was observed with a dual-role capable USB controller (e.g. Intel
>> > XHCI controller), which has the ability to switch role between host and device
>> > mode. The gadget is deleted when we switch role to device mode from host
>> > mode. See below log:
>> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
>>
>> oh, so you're using a modified tree :-) Then we can't really help.
>>
>> > [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
>> > [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
>> > [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
>> > [   41.191192] usb 1-1: USB disconnect, device number 3
>> > [   41.191284] usb 1-1.1: USB disconnect, device number 4
>> > [   41.218958] usb 1-1.5: USB disconnect, device number 5
>> > [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
>> > [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
>>
>> What is this android_work. That doesn't exist upstream.
>>
>> > [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
>> > [   41.263413] configfs-gadget gadget: unbind function 'Function FS
>> > Gadget'/ffff8801db049e38
>> > [   41.263969] configfs-gadget gadget: unbind function
>> > 'cdc_network'/ffff8801d8897400
>> > [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
>> > [   41.720957] dabr_udc deleted
>> > [   41.721097] dabridge 1-5 deleted
>> >
>> > The UDC and gadget will be deleted after switch role to device mode.
>> > And they will be
>> > created as new object when switching back to host mode. At this time
>> > the bind in function
>> > driver (e.g. f_ncm) will not set the new gadget.
>> >
>> > For kernel 4.19+, the role switch command will be:
>> >   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
>> >
>> > The latest Intel role switch kernel driver can be found here:
>> >   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c
>>
>> Right, please test against v5.2-rc5 and show me the problem on that
>> kernel. I can't apply patches for problems that may not even exist in
>> upstream, sorry.
>>
>> --
>> balbi
>
> The issue exist in main line kernel, but I can not test it with
> v5.2-rc5 kernel. I tested it with 4.19 kernel,

which of the v4.19?

> which, for the usb gadget part, has almost the same code as v5.2. It
> is 100% reproducible with dual role
> USB controller or by removing UDC hardware. Take f_ncm for example,
> the use case as follows:

Keep in mind that the way android handles dual-role is completely
different from what we have upstream.

> 1. USB controller is in host mode, f_ncm and UDC is configured in configfs.
>    - The ncm is instanced and alloced when "functions/ncm.usb0" is
> created and it will be freed
>       when those files are delted from configfs.
>
> 2. enable the gadget and bind it to this ncm function.
>     - For the first time running, ncm_opts->bound is 0 and
> gether_set_gadget is called to set the
>       gadget. The bound is set to 1 then.
>
> 3. If the UDC is disconnected from bus, then the UDC and its gadget is

what do you mean by "disconnected from the bus"? Removing the cable
(aka, disconnect) will only cause the ->disconnect() callback to be
called. It will not result in the UDC being freed. Is this, perhaps,
something specific to android?

> deleted. But because the
>     ncm.usb0 is still there, ncm object is not freed and
> ncm_opts->bound is still set.
>     There are two ways to disconnect the UDC hardware. One is for dual
> role host controller by switch
>     host controller role to device mode. Another way is by removing
> the UDC hardware from bus, both
>     will generate an usb device disconnect event to UDC driver to
> delete udc and gadget.

not true, unless I misunderstand what you mean. Disconnect will generate
a disconnect interrupt in most UDCs (except for dummy) and the
->disconnect() callback will be called. Nothing will be freed.

> 4. Now the bound of ncm is still set and gadget is deleted due to udc
> disconnected. And if we connect
>     the udc device again, then it will create new udc and gadget and
> bind to ncm again. But because
>     bound is already set, the new gadget is not set to gether
> (gether_register_netdev not called).
>
> Not sure if this is clear to you. Please review the scenario and the patch.

This sounds a little like it's android-specific. Is your platform using
dwc3? Can you capture tracepoints of the failure? ftrace_dump_on_oops
will help getting the actual tracepoints in this case.

cheers

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Lianwei Wang]

On Tue, Jun 18, 2019 at 11:21 PM Felipe Balbi <balbi@kernel.org> wrote:
>
>
> Hi,
>
> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
> >> >>
> >> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> >>
> >> >> > The udc and gadget device will be deleted when udc device is
> >> >> > disconnected and the related function will be unbind with it.
> >> >> >
> >> >> > But if the configfs is not deleted, then the function object
> >> >> > will be kept and the bound status is kept true.
> >> >> >
> >> >> > Then after udc device is connected again and a new udc and
> >> >> > gadget objects will be created and passed to bind interface.
> >> >> > But because the bound is still true, the new gadget is not
> >> >> > updated to netdev and a previous freed gadget will be used
> >> >> > in netdev after bind.
> >> >> >
> >> >> > To fix this using after freed issue, always set the gadget
> >> >> > object to netdev in bind interface.
> >> >> >
> >> >> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
> >> >>
> >> >> I can't actually understand what's the problem here. The gadget is not
> >> >> deleted when we disconnect the cable.
> >> >>
> >> >> --
> >> >> balbi
> >> >
> >> > The issue was observed with a dual-role capable USB controller (e.g. Intel
> >> > XHCI controller), which has the ability to switch role between host and device
> >> > mode. The gadget is deleted when we switch role to device mode from host
> >> > mode. See below log:
> >> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
> >>
> >> oh, so you're using a modified tree :-) Then we can't really help.
> >>
> >> > [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
> >> > [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
> >> > [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
> >> > [   41.191192] usb 1-1: USB disconnect, device number 3
> >> > [   41.191284] usb 1-1.1: USB disconnect, device number 4
> >> > [   41.218958] usb 1-1.5: USB disconnect, device number 5
> >> > [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
> >> > [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
> >>
> >> What is this android_work. That doesn't exist upstream.
> >>
> >> > [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
> >> > [   41.263413] configfs-gadget gadget: unbind function 'Function FS
> >> > Gadget'/ffff8801db049e38
> >> > [   41.263969] configfs-gadget gadget: unbind function
> >> > 'cdc_network'/ffff8801d8897400
> >> > [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
> >> > [   41.720957] dabr_udc deleted
> >> > [   41.721097] dabridge 1-5 deleted
> >> >
> >> > The UDC and gadget will be deleted after switch role to device mode.
> >> > And they will be
> >> > created as new object when switching back to host mode. At this time
> >> > the bind in function
> >> > driver (e.g. f_ncm) will not set the new gadget.
> >> >
> >> > For kernel 4.19+, the role switch command will be:
> >> >   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
> >> >
> >> > The latest Intel role switch kernel driver can be found here:
> >> >   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >>
> >> Right, please test against v5.2-rc5 and show me the problem on that
> >> kernel. I can't apply patches for problems that may not even exist in
> >> upstream, sorry.
> >>
> >> --
> >> balbi
> >
> > The issue exist in main line kernel, but I can not test it with
> > v5.2-rc5 kernel. I tested it with 4.19 kernel,
>
> which of the v4.19?
>
> > which, for the usb gadget part, has almost the same code as v5.2. It
> > is 100% reproducible with dual role
> > USB controller or by removing UDC hardware. Take f_ncm for example,
> > the use case as follows:
>
> Keep in mind that the way android handles dual-role is completely
> different from what we have upstream.
>
Right, Our xchi controller support dual role and normally it works in host mode
and the other device, e.g. phone can connect to our system as a gadget device.
It works in the same way as PC.

> > 1. USB controller is in host mode, f_ncm and UDC is configured in configfs.
> >    - The ncm is instanced and alloced when "functions/ncm.usb0" is
> > created and it will be freed
> >       when those files are delted from configfs.
> >
> > 2. enable the gadget and bind it to this ncm function.
> >     - For the first time running, ncm_opts->bound is 0 and
> > gether_set_gadget is called to set the
> >       gadget. The bound is set to 1 then.
> >
> > 3. If the UDC is disconnected from bus, then the UDC and its gadget is
>
> what do you mean by "disconnected from the bus"? Removing the cable
> (aka, disconnect) will only cause the ->disconnect() callback to be
> called. It will not result in the UDC being freed. Is this, perhaps,
> something specific to android?
>
Our UDC is removable so we can remove the UDC from our system and the
udc and gadget will be deleted after that.

And our xhci support dual role mode and we can switch role between host and
device mode. In host mode, the UDC will be connected to xhci host controller
and the other device can connect to UDC. In device mode, it will work
as a device
and can connect to other host computer.

> > deleted. But because the
> >     ncm.usb0 is still there, ncm object is not freed and
> > ncm_opts->bound is still set.
> >     There are two ways to disconnect the UDC hardware. One is for dual
> > role host controller by switch
> >     host controller role to device mode. Another way is by removing
> > the UDC hardware from bus, both
> >     will generate an usb device disconnect event to UDC driver to
> > delete udc and gadget.
>
> not true, unless I misunderstand what you mean. Disconnect will generate
> a disconnect interrupt in most UDCs (except for dummy) and the
> ->disconnect() callback will be called. Nothing will be freed.
>
As I mentioned above, the disconnect means the UDC is disconnected.

> > 4. Now the bound of ncm is still set and gadget is deleted due to udc
> > disconnected. And if we connect
> >     the udc device again, then it will create new udc and gadget and
> > bind to ncm again. But because
> >     bound is already set, the new gadget is not set to gether
> > (gether_register_netdev not called).
> >
> > Not sure if this is clear to you. Please review the scenario and the patch.
>
> This sounds a little like it's android-specific. Is your platform using
> dwc3? Can you capture tracepoints of the failure? ftrace_dump_on_oops
> will help getting the actual tracepoints in this case.
>
> cheers
>
It is not android specific. For the dual role controller, in device
mode the dwc3 will
work as gadget device and can connect to PC as device. In host mode, the dwc3
does not work, instead the removable UDC will be connected to our system as
gadget device.

> --
> balbi

And the key point is that if the UDC can be disconnected as deleted?
The answer is
yes for below two cases:
  - The UDC is removable
  - It is a dual role controller and can switch between host mode and
device mode. In host
mode it will work as a xhci host controller. And in device mode it
works as dwc3 gadget
device.

In both way we see that the udc is disconnected and deleted in our system.

config USB_ROLES_INTEL_XHCI
        tristate "Intel XHCI USB Role Switch"
        depends on ACPI && X86
        help
          Driver for the internal USB role switch for switching the USB data
          lines between the xHCI host controller and the dwc3 gadget controller
          found on various Intel SoCs.

          To compile the driver as a module, choose M here: the module will
          be called intel-xhci-usb-role-switch.

The issue should be 100% reproducible with that support Intel XHCI USB
role switch, or
the udc can be removabled.

Thanks,
Lianwei

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Felipe Balbi]

Hi,

Lianwei Wang <lianwei.wang@gmail.com> writes:

> On Tue, Jun 18, 2019 at 11:21 PM Felipe Balbi <balbi@kernel.org> wrote:
>>
>>
>> Hi,
>>
>> Lianwei Wang <lianwei.wang@gmail.com> writes:
>> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
>> >> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
>> >> >>
>> >> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
>> >> >>
>> >> >> > The udc and gadget device will be deleted when udc device is
>> >> >> > disconnected and the related function will be unbind with it.
>> >> >> >
>> >> >> > But if the configfs is not deleted, then the function object
>> >> >> > will be kept and the bound status is kept true.
>> >> >> >
>> >> >> > Then after udc device is connected again and a new udc and
>> >> >> > gadget objects will be created and passed to bind interface.
>> >> >> > But because the bound is still true, the new gadget is not
>> >> >> > updated to netdev and a previous freed gadget will be used
>> >> >> > in netdev after bind.
>> >> >> >
>> >> >> > To fix this using after freed issue, always set the gadget
>> >> >> > object to netdev in bind interface.
>> >> >> >
>> >> >> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
>> >> >>
>> >> >> I can't actually understand what's the problem here. The gadget is not
>> >> >> deleted when we disconnect the cable.
>> >> >>
>> >> >> --
>> >> >> balbi
>> >> >
>> >> > The issue was observed with a dual-role capable USB controller (e.g. Intel
>> >> > XHCI controller), which has the ability to switch role between host and device
>> >> > mode. The gadget is deleted when we switch role to device mode from host
>> >> > mode. See below log:
>> >> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
>> >>
>> >> oh, so you're using a modified tree :-) Then we can't really help.
>> >>
>> >> > [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
>> >> > [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
>> >> > [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
>> >> > [   41.191192] usb 1-1: USB disconnect, device number 3
>> >> > [   41.191284] usb 1-1.1: USB disconnect, device number 4
>> >> > [   41.218958] usb 1-1.5: USB disconnect, device number 5
>> >> > [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
>> >> > [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
>> >>
>> >> What is this android_work. That doesn't exist upstream.
>> >>
>> >> > [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
>> >> > [   41.263413] configfs-gadget gadget: unbind function 'Function FS
>> >> > Gadget'/ffff8801db049e38
>> >> > [   41.263969] configfs-gadget gadget: unbind function
>> >> > 'cdc_network'/ffff8801d8897400
>> >> > [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
>> >> > [   41.720957] dabr_udc deleted
>> >> > [   41.721097] dabridge 1-5 deleted
>> >> >
>> >> > The UDC and gadget will be deleted after switch role to device mode.
>> >> > And they will be
>> >> > created as new object when switching back to host mode. At this time
>> >> > the bind in function
>> >> > driver (e.g. f_ncm) will not set the new gadget.
>> >> >
>> >> > For kernel 4.19+, the role switch command will be:
>> >> >   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
>> >> >
>> >> > The latest Intel role switch kernel driver can be found here:
>> >> >   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c
>> >>
>> >> Right, please test against v5.2-rc5 and show me the problem on that
>> >> kernel. I can't apply patches for problems that may not even exist in
>> >> upstream, sorry.
>> >>
>> >> --
>> >> balbi
>> >
>> > The issue exist in main line kernel, but I can not test it with
>> > v5.2-rc5 kernel. I tested it with 4.19 kernel,
>>
>> which of the v4.19?
>>
>> > which, for the usb gadget part, has almost the same code as v5.2. It
>> > is 100% reproducible with dual role
>> > USB controller or by removing UDC hardware. Take f_ncm for example,
>> > the use case as follows:
>>
>> Keep in mind that the way android handles dual-role is completely
>> different from what we have upstream.
>>
> Right, Our xchi controller support dual role and normally it works in host mode
> and the other device, e.g. phone can connect to our system as a gadget device.
> It works in the same way as PC.
>
>> > 1. USB controller is in host mode, f_ncm and UDC is configured in configfs.
>> >    - The ncm is instanced and alloced when "functions/ncm.usb0" is
>> > created and it will be freed
>> >       when those files are delted from configfs.
>> >
>> > 2. enable the gadget and bind it to this ncm function.
>> >     - For the first time running, ncm_opts->bound is 0 and
>> > gether_set_gadget is called to set the
>> >       gadget. The bound is set to 1 then.
>> >
>> > 3. If the UDC is disconnected from bus, then the UDC and its gadget is
>>
>> what do you mean by "disconnected from the bus"? Removing the cable
>> (aka, disconnect) will only cause the ->disconnect() callback to be
>> called. It will not result in the UDC being freed. Is this, perhaps,
>> something specific to android?
>>
> Our UDC is removable so we can remove the UDC from our system and the
> udc and gadget will be deleted after that.
>
> And our xhci support dual role mode and we can switch role between host and
> device mode. In host mode, the UDC will be connected to xhci host controller
> and the other device can connect to UDC. In device mode, it will work
> as a device
> and can connect to other host computer.
>
>> > deleted. But because the
>> >     ncm.usb0 is still there, ncm object is not freed and
>> > ncm_opts->bound is still set.
>> >     There are two ways to disconnect the UDC hardware. One is for dual
>> > role host controller by switch
>> >     host controller role to device mode. Another way is by removing
>> > the UDC hardware from bus, both
>> >     will generate an usb device disconnect event to UDC driver to
>> > delete udc and gadget.
>>
>> not true, unless I misunderstand what you mean. Disconnect will generate
>> a disconnect interrupt in most UDCs (except for dummy) and the
>> ->disconnect() callback will be called. Nothing will be freed.
>>
> As I mentioned above, the disconnect means the UDC is disconnected.
>
>> > 4. Now the bound of ncm is still set and gadget is deleted due to udc
>> > disconnected. And if we connect
>> >     the udc device again, then it will create new udc and gadget and
>> > bind to ncm again. But because
>> >     bound is already set, the new gadget is not set to gether
>> > (gether_register_netdev not called).
>> >
>> > Not sure if this is clear to you. Please review the scenario and the patch.
>>
>> This sounds a little like it's android-specific. Is your platform using
>> dwc3? Can you capture tracepoints of the failure? ftrace_dump_on_oops
>> will help getting the actual tracepoints in this case.
>>
>> cheers
>>
> It is not android specific. For the dual role controller, in device
> mode the dwc3 will
> work as gadget device and can connect to PC as device. In host mode, the dwc3
> does not work, instead the removable UDC will be connected to our system as
> gadget device.
>
>> --
>> balbi
>
> And the key point is that if the UDC can be disconnected as deleted?
> The answer is
> yes for below two cases:
>   - The UDC is removable
>   - It is a dual role controller and can switch between host mode and
> device mode. In host
> mode it will work as a xhci host controller. And in device mode it
> works as dwc3 gadget
> device.
>
> In both way we see that the udc is disconnected and deleted in our system.
>
> config USB_ROLES_INTEL_XHCI
>         tristate "Intel XHCI USB Role Switch"
>         depends on ACPI && X86
>         help
>           Driver for the internal USB role switch for switching the USB data
>           lines between the xHCI host controller and the dwc3 gadget controller
>           found on various Intel SoCs.
>
>           To compile the driver as a module, choose M here: the module will
>           be called intel-xhci-usb-role-switch.
>
> The issue should be 100% reproducible with that support Intel XHCI USB
> role switch, or
> the udc can be removabled.

what's specific to your case is *how* you implemented dual-role. None of
that is upstream and I have no means of verifying that what you claim is
true.

Sorry

-- 
balbi

Re: [PATCH] usb: gadget: avoid using gadget after freed

[Lianwei Wang]

On Wed, Jun 19, 2019 at 10:55 PM Felipe Balbi <balbi@kernel.org> wrote:
>
>
> Hi,
>
> Lianwei Wang <lianwei.wang@gmail.com> writes:
>
> > On Tue, Jun 18, 2019 at 11:21 PM Felipe Balbi <balbi@kernel.org> wrote:
> >>
> >>
> >> Hi,
> >>
> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> >> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi <balbi@kernel.org> wrote:
> >> >> >>
> >> >> >> Lianwei Wang <lianwei.wang@gmail.com> writes:
> >> >> >>
> >> >> >> > The udc and gadget device will be deleted when udc device is
> >> >> >> > disconnected and the related function will be unbind with it.
> >> >> >> >
> >> >> >> > But if the configfs is not deleted, then the function object
> >> >> >> > will be kept and the bound status is kept true.
> >> >> >> >
> >> >> >> > Then after udc device is connected again and a new udc and
> >> >> >> > gadget objects will be created and passed to bind interface.
> >> >> >> > But because the bound is still true, the new gadget is not
> >> >> >> > updated to netdev and a previous freed gadget will be used
> >> >> >> > in netdev after bind.
> >> >> >> >
> >> >> >> > To fix this using after freed issue, always set the gadget
> >> >> >> > object to netdev in bind interface.
> >> >> >> >
> >> >> >> > Signed-off-by: Lianwei Wang <lianwei.wang@gmail.com>
> >> >> >>
> >> >> >> I can't actually understand what's the problem here. The gadget is not
> >> >> >> deleted when we disconnect the cable.
> >> >> >>
> >> >> >> --
> >> >> >> balbi
> >> >> >
> >> >> > The issue was observed with a dual-role capable USB controller (e.g. Intel
> >> >> > XHCI controller), which has the ability to switch role between host and device
> >> >> > mode. The gadget is deleted when we switch role to device mode from host
> >> >> > mode. See below log:
> >> >> > # echo p > /sys/devices/pci0000:00/0000:00:15.1/intel-cht-otg.0/mux_state #(4.4)
> >> >>
> >> >> oh, so you're using a modified tree :-) Then we can't really help.
> >> >>
> >> >> > [   41.170891] intel-cht-otg intel-cht-otg.0: p: set PERIPHERAL mode
> >> >> > [   41.171895] dwc3 dwc3.0.auto: DWC3 OTG Notify USB_EVENT_VBUS
> >> >> > [   41.187420] dwc3 dwc3.0.auto: dwc3_resume_common
> >> >> > [   41.191192] usb 1-1: USB disconnect, device number 3
> >> >> > [   41.191284] usb 1-1.1: USB disconnect, device number 4
> >> >> > [   41.218958] usb 1-1.5: USB disconnect, device number 5
> >> >> > [   41.238117] android_work: sent uevent USB_STATE=CONFIGURED
> >> >> > [   41.240572] android_work: sent uevent USB_STATE=DISCONNECTED
> >> >>
> >> >> What is this android_work. That doesn't exist upstream.
> >> >>
> >> >> > [   41.263285] platform dabr_udc.0: unregister gadget driver 'configfs-gadget'
> >> >> > [   41.263413] configfs-gadget gadget: unbind function 'Function FS
> >> >> > Gadget'/ffff8801db049e38
> >> >> > [   41.263969] configfs-gadget gadget: unbind function
> >> >> > 'cdc_network'/ffff8801d8897400
> >> >> > [   41.325943] dabridge 1-1.5:1.0: Port 3 VBUS OFF
> >> >> > [   41.720957] dabr_udc deleted
> >> >> > [   41.721097] dabridge 1-5 deleted
> >> >> >
> >> >> > The UDC and gadget will be deleted after switch role to device mode.
> >> >> > And they will be
> >> >> > created as new object when switching back to host mode. At this time
> >> >> > the bind in function
> >> >> > driver (e.g. f_ncm) will not set the new gadget.
> >> >> >
> >> >> > For kernel 4.19+, the role switch command will be:
> >> >> >   echo "device" > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
> >> >> >
> >> >> > The latest Intel role switch kernel driver can be found here:
> >> >> >   https://elixir.bootlin.com/linux/v5.2-rc5/source/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >> >>
> >> >> Right, please test against v5.2-rc5 and show me the problem on that
> >> >> kernel. I can't apply patches for problems that may not even exist in
> >> >> upstream, sorry.
> >> >>
> >> >> --
> >> >> balbi
> >> >
> >> > The issue exist in main line kernel, but I can not test it with
> >> > v5.2-rc5 kernel. I tested it with 4.19 kernel,
> >>
> >> which of the v4.19?
> >>
> >> > which, for the usb gadget part, has almost the same code as v5.2. It
> >> > is 100% reproducible with dual role
> >> > USB controller or by removing UDC hardware. Take f_ncm for example,
> >> > the use case as follows:
> >>
> >> Keep in mind that the way android handles dual-role is completely
> >> different from what we have upstream.
> >>
> > Right, Our xchi controller support dual role and normally it works in host mode
> > and the other device, e.g. phone can connect to our system as a gadget device.
> > It works in the same way as PC.
> >
> >> > 1. USB controller is in host mode, f_ncm and UDC is configured in configfs.
> >> >    - The ncm is instanced and alloced when "functions/ncm.usb0" is
> >> > created and it will be freed
> >> >       when those files are delted from configfs.
> >> >
> >> > 2. enable the gadget and bind it to this ncm function.
> >> >     - For the first time running, ncm_opts->bound is 0 and
> >> > gether_set_gadget is called to set the
> >> >       gadget. The bound is set to 1 then.
> >> >
> >> > 3. If the UDC is disconnected from bus, then the UDC and its gadget is
> >>
> >> what do you mean by "disconnected from the bus"? Removing the cable
> >> (aka, disconnect) will only cause the ->disconnect() callback to be
> >> called. It will not result in the UDC being freed. Is this, perhaps,
> >> something specific to android?
> >>
> > Our UDC is removable so we can remove the UDC from our system and the
> > udc and gadget will be deleted after that.
> >
> > And our xhci support dual role mode and we can switch role between host and
> > device mode. In host mode, the UDC will be connected to xhci host controller
> > and the other device can connect to UDC. In device mode, it will work
> > as a device
> > and can connect to other host computer.
> >
> >> > deleted. But because the
> >> >     ncm.usb0 is still there, ncm object is not freed and
> >> > ncm_opts->bound is still set.
> >> >     There are two ways to disconnect the UDC hardware. One is for dual
> >> > role host controller by switch
> >> >     host controller role to device mode. Another way is by removing
> >> > the UDC hardware from bus, both
> >> >     will generate an usb device disconnect event to UDC driver to
> >> > delete udc and gadget.
> >>
> >> not true, unless I misunderstand what you mean. Disconnect will generate
> >> a disconnect interrupt in most UDCs (except for dummy) and the
> >> ->disconnect() callback will be called. Nothing will be freed.
> >>
> > As I mentioned above, the disconnect means the UDC is disconnected.
> >
> >> > 4. Now the bound of ncm is still set and gadget is deleted due to udc
> >> > disconnected. And if we connect
> >> >     the udc device again, then it will create new udc and gadget and
> >> > bind to ncm again. But because
> >> >     bound is already set, the new gadget is not set to gether
> >> > (gether_register_netdev not called).
> >> >
> >> > Not sure if this is clear to you. Please review the scenario and the patch.
> >>
> >> This sounds a little like it's android-specific. Is your platform using
> >> dwc3? Can you capture tracepoints of the failure? ftrace_dump_on_oops
> >> will help getting the actual tracepoints in this case.
> >>
> >> cheers
> >>
> > It is not android specific. For the dual role controller, in device
> > mode the dwc3 will
> > work as gadget device and can connect to PC as device. In host mode, the dwc3
> > does not work, instead the removable UDC will be connected to our system as
> > gadget device.
> >
> >> --
> >> balbi
> >
> > And the key point is that if the UDC can be disconnected as deleted?
> > The answer is
> > yes for below two cases:
> >   - The UDC is removable
> >   - It is a dual role controller and can switch between host mode and
> > device mode. In host
> > mode it will work as a xhci host controller. And in device mode it
> > works as dwc3 gadget
> > device.
> >
> > In both way we see that the udc is disconnected and deleted in our system.
> >
> > config USB_ROLES_INTEL_XHCI
> >         tristate "Intel XHCI USB Role Switch"
> >         depends on ACPI && X86
> >         help
> >           Driver for the internal USB role switch for switching the USB data
> >           lines between the xHCI host controller and the dwc3 gadget controller
> >           found on various Intel SoCs.
> >
> >           To compile the driver as a module, choose M here: the module will
> >           be called intel-xhci-usb-role-switch.
> >
> > The issue should be 100% reproducible with that support Intel XHCI USB
> > role switch, or
> > the udc can be removabled.
>
> what's specific to your case is *how* you implemented dual-role. None of
> that is upstream and I have no means of verifying that what you claim is
> true.
>
> Sorry
>
> --
> balbi

That's Ok, Maybe I'm misunderstanding how upstream kernel works.

The issue is happened on Intel hardware which should be support by
upstream kernel.

I did not implement any dual role, it is from upstream. And the UDC driver
is working as the same mechanism as upstream too. The upstream kernel
support remove/disconnect UDC device.

I try to explain it clearly but unfortunately... my failure. If we
look at the code,
we have remove function defined in udc driver, and which will finally delete UDC
(usb_del_gadget_udc).

Could you tell me how the upstream kernel guarantee that the remove and
usb_del_gadget_udc function is never called?

Thanks,
Lianwei