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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877guxhx44.fsf@nemi.mork.no>
Date:	Sun, 24 Jun 2012 11:34:19 +0200
From:	Bjørn Mork <bjorn@...k.no>
To:	Oliver Neukum <oliver@...kum.org>
Cc:	Ming Lei <tom.leiming@...il.com>, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org,
	Marius Bjørnstad Kotsbak 
	<marius.kotsbak@...il.com>
Subject: Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting

Oliver Neukum <oliver@...kum.org> writes:
> Am Samstag, 23. Juni 2012, 17:32:09 schrieb Bjørn Mork:
>> Ming Lei <tom.leiming@...il.com> writes:
>> > On Sat, Jun 23, 2012 at 4:45 PM, Bjørn Mork <bjorn@...k.no> wrote:
>
>> > Suppose there will be another usbnet driver which has its own subdriver
>> > too, the same trick of checking need to be added again if not taking the
>> > general way of simply removing 'usb_set_intfdata(intf, NULL);' in
>> > usbnet_disconnect.
>> 
>> Yes, I guess so.
>> 
>> I am just worrying (maybe too much) about the unknown consequences of
>> removing that code in usbnet, not fully understanding why it was there
>> in the first place.  And I do not want to take the blame and cleanup
>> work if anything goes wrong :-) Fixing it in qmi_wwan feels much safer.
>
> void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
>         struct cdc_state                *info = (void *) &dev->data;
>         struct usb_driver               *driver = driver_of(intf);
>
>         /* disconnect master --> disconnect slave */
>         if (intf == info->control && info->data) {
>                 /* ensure immediate exit from usbnet_disconnect */
>                 usb_set_intfdata(info->data, NULL);
>                 usb_driver_release_interface(driver, info->data);
>                 info->data = NULL;
>
> 1. We mirror the minidrivers closely, which reduces errors
> 2. unbind() is called with the data anyway and after disconnect()
>     the intfdata is not valid anyway, because the interface may have been
>     reprobed.

Sorry, I did not understand what you meant we should do here.  The extra
usb_set_intfdata(, NULL) in usbnet_disconnect() won't make any
difference for that piece of code, will it?

And the USB core ensures that intfdata is set to NULL before any
reprobing, so that will never be a problem.  That's the reason why it
seems redundant setting it in usbnet_disconnect().



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