netdev - Re: [PATCH] usbnet: prevent device rpm suspend in usbnet

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: <8760nwj0l3.fsf@miraculix.mork.no>
Date:   Wed, 09 Nov 2016 13:32:08 +0100
From:   Bjørn Mork <bjorn@...k.no>
To:     Oliver Neukum <oneukum@...e.com>
Cc:     Alan Stern <stern@...land.harvard.edu>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Oliver Neukum <oneukum@...e.com> writes:

> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>
>> These problems could very well be caused by running at SuperSpeed
>> (USB-3) instead of high speed (USB-2).
>> 
>> Is there any way to test what happens when the device is attached to 
>> the computer by a USB-2 cable?  That would prevent it from operating at 
>> SuperSpeed.
>> 
>> The main point, however, is that the proposed patch doesn't seem to
>> address the true problem, which is that the device gets suspended
>> between probes.  The patch only tries to prevent it from being
>> suspended during a probe -- which is already prevented by the USB core.
>
> But why doesn't it fail during normal operation?
>
> I suspect that its firmware requires the altsetting
>
>         /* should we change control altsetting on a NCM/MBIM function? */
>         if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
>                 data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
>                 ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM);
>
> to be set before it accepts a suspension.

Could be, but I don't think so.  The above code is effectively a noop
unless the function is a combined NCM/MBIM function.  Something I've
never seen on a Sierra Wireless device (ignoring the infamous EM7345,
which really is an Intel device).

This is a typical example of a Sierra Wireless modem configured for
MBIM:

P:  Vendor=1199 ProdID=9079 Rev= 0.06
S:  Manufacturer=Sierra Wireless, Incorporated
S:  Product=Sierra Wireless EM7455 Qualcomm Snapdragon X7 LTE-A
S:  SerialNumber=LF615126xxxxxxx
C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=500mA
A:  FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=(none)
E:  Ad=82(I) Atr=03(Int.) MxPS=  64 Ivl=32ms
I:* If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
I:  If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


The control interface of plain MBIM functions will always have a single
altsetting, like the example above. So cdc_ncm_select_altsetting(intf)
returns "0", while CDC_NCM_COMM_ALTSETTING_MBIM is "1".


Just for reference, using the Intel^H^H^H^H^HEM7345 as example, this is
what a combined NCM/MBIM function looks like:


P:  Vendor=1199 ProdID=a001 Rev=17.29
S:  Manufacturer=Sierra Wireless Inc.
S:  Product=Sierra Wireless EM7345 4G LTE
S:  SerialNumber=013937000xxxxxx
C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=100mA
A:  FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0d Prot=00
A:  FirstIf#= 2 IfCount= 2 Cls=02(comm.) Sub=02 Prot=01
I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
E:  Ad=81(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:  If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
E:  Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=01 Driver=(none)
E:  Ad=83(I) Atr=03(Int.) MxPS=  64 Ivl=1ms
I:* If#= 3 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E:  Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


And this is what the code you quote is trying to deal with.  Note the
different subclass of altsetting 0 and 1.... This is incredibly ugly.

FWIW, the modem in question cannot be an EM7345. That modem does not
have the static interface numbering oddity.  Another sign that it isn't
a true Sierra device.




Bjørn