linux-kernel - Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed

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]

Date:	Thu, 5 May 2016 18:35:52 +0800
From:	Jim Lin <jilin@...dia.com>
To:	Felipe Balbi <balbi@...nel.org>
CC:	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed

On 2016年05月04日 18:37, Felipe Balbi wrote:
> * PGP Signed by an unknown key
>
>
> Hi,
>
> Jim Lin <jilin@...dia.com> writes:
>
> <snip>
>
>>>> In f_fs.c
>>>> "
>>>> static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
>>>>                     struct usb_os_desc_header *h, void *data,
>>>>                     unsigned len, void *priv)
>>>> {
>>>>        struct ffs_data *ffs = priv;
>>>>        u8 length;
>>>>
>>>>        ENTER();
>>>>
>>>>        switch (type) {
>>>>        case FFS_OS_DESC_EXT_COMPAT: {
>>>>            struct usb_ext_compat_desc *d = data;
>>>>            int i;
>>>>
>>>>            if (len < sizeof(*d) ||
>>>>                d->bFirstInterfaceNumber >= ffs->interfaces_count ||
>>>>                d->Reserved1)
>>>>                return -EINVAL;
>>>> "
>>> that's fine, but this is only failing because something else is
>>> returning the wrong set of descriptors (SS vs HS). That's the bug we
>>> want to fix, not work around it.
>>>
>> Thanks.
> you're welcome, but to fix that bug we need more information. Why is
> composite.c using the wrong set of descriptors ? What is your setup ?
>
> Are you using an in-kernel gadget ? which one ?
No, our gadget driver is on the way to submit.
> Using configfs or legacy
> gadgets ? gadgetfs ? f_fs ?

>   How to trigger this ? Can you provide
> instructions and (in case of gadgetfs/ffs) code to create a gadget that
> hits this problem ?
>
Please refer to
https://android.googlesource.com/platform/system/core/+/master/adb/usb_linux_client.cpp
https://android.googlesource.com/device/google/dragon/+/android-6.0.1_r4/init.dragon.usb.rc
https://android.googlesource.com/platform/system/core/+/master/rootdir/init.usb.configfs.rc

Also this is a thought coming from another engineer for your reference.
"

I think Microsoft and linux are contradicting the requirements. 
According MSFT's os descriptor definition, one of the reserved fields 
needs to be set to 1 whereas seems like f_fs.c expects them to be 0. 
(copy pasting from the spec downloaded from: 
https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx) 
What does upstream think ? Requires some conflict resolution I guess !! 
Since the OS descriptors are from MSFT, I believe upstream has to drop 
the check and I think this patch might be valid..

bFirstInterfaceNumber This field specifies the interface or function 
that is associated with the IDs in this section. To use this function 
section to associate a single-function group of interfaces with a single 
pair of IDs, set bFirstInterfaceNumber to the first interface in the 
group. Then use an IAD in that interface’s interface descriptor to 
specify which additional interfaces should be included in the group. The 
interfaces in the group must be consecutively numbered. For details, see 
“Support for USB Interface Association Descriptor in Windows.”

RESERVED Reserved for system use. Set this value to 0x01.

compatibleID This field contains the value of the compatible ID to be 
associated with this function. Any unused bytes should be filled with 
NULLs. If the function does not have a compatible ID, fill the entire 
field with NULLs.

subCompatibleID This field contains the value of the subcompatible ID to 
be associated with this function. Any remaining bytes should be filled 
with NULLs. If the function does not have a subcompatible ID, fill the 
entire field with NULLs.

RESERVED Reserved. Fill this value with NULLs.
"

--nvpublic