Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting

[Bjørn Mork <bjorn@xxxxxxx>]

Ming Lei <tom.leiming@xxxxxxxxx> writes:

> On Sat, Jun 23, 2012 at 12:18 AM, Bjørn Mork <bjorn@xxxxxxx> wrote:
>> Ming Lei <tom.leiming@xxxxxxxxx> writes:
>
>> Yes, intfdata is per interface, but in the case of cdc_ether (and
>> probably other similar minidrivers) there are two interfaces pointing to
>> the *same* usbnet private data.
>
> Yes, the priv pointers of both interface points to the usbnet instance, but
> the same pointer is stored into two places.
>
>>
>> What about the situation where disconnect is called simultaneously for
>> both interfaces? Or can't that happen? If it can, then we'll do
>
> It can't happen because both parent lock and its lock need to be held in
> hotplug situation or unbind situation.

OK, then it can probably go away.  But then again it doesn't do any harm
for any other minidriver.  qmi_wwan is special because of the
cooperation with cdc_wdm.

>>  driver->disconnect(intf1)                driver->disconnect(intf2)
>>     dev = usb_get_intfdata(intf1)           dev = usb_get_intfdata(intf2)
>>     dev->driver_info->unbind()              dev->driver_info->unbind()
>>     net = dev->net                          net = dev->net
>>     free_netdev(net)                        free_netdev(dev->net)
>>
>> where "dev" and "net" will be pointing to the same private data and
>> netdevice.
>
> Suppose driver->disconnect(intf1) is called first, .ubind() inside
> .disconnect will clear intfdata of another interface(intf2) and call
> usb_driver_release_interface(intf2), which will cause .disconnect(intf2)
> called, but it will return immediately.

Yes, if that's the only possible call sequence then I agree that
removing the call won't be a problem.  But I don't see much gain either.
And it has been there since the beginning of git history, if that counts
for anything around here.

>> I assume that is what the code above is trying to protect against.  But
>
> I don't see the protection in the code, :-)

I am trying my best to be positive :-)


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html