[PATCH v2] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
This fixes a regression which was introduced by commit f1bddbb, by
reverting a small fragment of commit 855ed04.
If the following conditions were met, usb_gadget_probe_driver() returned
0, although the call was unsuccessful:
1. A particular UDC was specified by thge gadget driver (using member
"udc_name" of struct usb_gadget_driver).
2. The UDC with this name is available.
3. Another gadget driver is already bound to this gadget.
4. The gadget driver has the "match_existing_only" flag set.
In this case, the return code variable "ret" is set to 0, the return
code of a strcmp() call (to check for the second condition).
This also fixes an oops which could occur in the following scenario:
1. Two usb gadget instances were configured using configfs.
2. The first gadget configuration was bound to a UDC (using the configfs
attribute "UDC").
3. It was tried to bind the second gadget configuration to the same UDC
in the same way. This operation was then wrongly reported as being
successful.
4. The second gadget configuration's "UDC" attribute is cleared, to
unbind the (not really bound) second gadget configuration from the UDC.
] __list_del_entry+0x29/0xc0
PGD 41b4c5067
PUD 41a598067
PMD 0
Oops: [#1] SMP
Modules linked in: cdc_acm usb_f_fs usb_f_serial
usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl
x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm
snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul
ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper
ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw
uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel
videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm
videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth
snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev
media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm
snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core
lpc_ich fat
efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev
intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core
tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq
hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas
usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel
i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci
drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core
fjes button [last unloaded: net2280]
CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77
Extreme3, BIOS P1.50 07/11/2013
task: 880419ce4040 task.stack: c90002ed4000
RIP: 0010:[] []
__list_del_entry+0x29/0xc0
RSP: 0018:c90002ed7d68 EFLAGS: 00010207
RAX: RBX: 88041787ec30 RCX: dead0200
RDX: RSI: 880417482002 RDI: 88041787ec30
RBP: c90002ed7d68 R08: R09: 0010
R10: R11: 880419ce4040 R12: 88041787eb68
R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001
FS: 7fe4e49b8700() GS:88042f34()
knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: CR3: 00041b4c4000 CR4: 001406e0
Stack:
c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0
c0ae22aa 88041787e800 88041787e800 c90002ed7dc0
c0d7a727 952273fa 88041aba5760 c90002ed7df8
Call Trace:
[] list_del+0xd/0x30
[] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core]
[] unregister_gadget+0x27/0x60 [libcomposite]
[] ? mutex_lock+0x1a/0x30
[] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite]
[] configfs_write_file+0xa0/0x100 [configfs]
[] __vfs_write+0x37/0x160
[] ? __fd_install+0x30/0xd0
[] ? _raw_spin_unlock+0xe/0x10
[] vfs_write+0xb8/0x1b0
[] SyS_write+0x58/0xc0
[] ? __close_fd+0x94/0xc0
[] entry_SYSCALL_64_fastpath+0x1e/0xad
Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89
e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b
02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08
RIP [] __list_del_entry+0x29/0xc0
RSP
CR2:
---[ end trace 99fc090ab3ff6cbc ]---
Fixes: f1bddbb ("usb: gadget: Fix binding to UDC via configfs
interface")
Signed-off-by: Felix Hädicke
Tested-by: Krzysztof Opasiak
---
changes in v2:
- added "Fixes:" tag for commit f1bddbb
- added "Tested-by:" tag for Krzysztof Opasiak
drivers/usb/gadget/udc/core.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 9483489080f6..0402177f93cd 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1317,7 +1317,11 @@ int usb_gadget_probe_driver(struct usb_gadg
Aw: Re: [PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
Hi,
> On 12/01/2016 10:44 PM, Felix Hädicke wrote:
> > Hi,
> >
> >> Hi,
> >>
> >> Good catch but I have a few comments to commit message:
> >>
> >> On 12/01/2016 12:24 AM, Felix Hädicke wrote:
> >>> This fixes a regression which was introduced by commit f1bddbb, by
> >>> reverting a small fragment of commit 855ed04.
> >>>
> >> Not exactly. The regression has been introduced by commit 855ed04
> >> itself.This commit replaced previous construction similar what you sent
> >> now with simple if():
> >>
> >> @@ -535,11 +556,7 @@ int usb_gadget_probe_driver(struct
> >> usb_gadget_driver *driver)
> >> if (!ret)
> >> break;
> >> }
> >> - if (ret)
> >> - ret = -ENODEV;
> >> - else if (udc->driver)
> >> - ret = -EBUSY;
> >> - else
> >> + if (!ret && !udc->driver)
> >> goto found;
> >> } else {
> >> list_for_each_entry(udc, &udc_list, list) {
> >
> > The regression bug I want to fix with this patch was introduced with
> > commit f1bddbb, not 855ed04. With 855ed04, the this if/else construct
> > was changed. But concerning the usb_gadget_probe_driver() return code,
> > this was ok.
>
> Nope. Return code was 0 and gadget has not been bound to any udc but
> added to pending list. Consider the following sequence:
>
> echo "udc" > g1/UDC
> echo "udc" > g2/UDC
> echo "" > g1/UDC
>
> The expected result is that second line should fail with EBUSY. And
> without f1bddbb the returned value will be 0 and g2 will be pending on a
> list. After line 3 udc will be free but g2 will still be hanging on a
> pending list.
As I understand 855ed04, it is the new expected behaviour that
usb_gadget_probe_driver() returns 0, even if there is no UDC (with the
given name) available yet, or another gadget driver is already active.
So there is no bug here (at least none concerning the
usb_gadget_probe_driver() return code).
But with commit f1bddbb, there is a new code path in which a strcmp()
return code is returned from usb_gadget_probe_driver(), which does not
make sense.
And the issue you are describing is still there, my patch does not fix it:
1. rmmod
2. modprobe
2. modprobe
3. modprobe
The first gadget driver is now active.
4. rmmod
-> The second driver is not activated (although it is in the pending list).
>
> > There was no code path were a value which was not meant to
> > be the function return code was accidentally returned. With commit
> > f1bddbb, and the introduction of a new code path for the flag
> > match_existing_only, this changed.
> >
>
> This flag changed the error from "leave it on the list forever when udc
> is busy" to "NULL ptr dereference" which is obviously much worse.
>
> >> After that commit, if UDC is busy, gadget is added to the list of
> >> pending gadgets. Unfortunately this list is not rescanned in
> >> usb_gadget_unregister_driver(). This means that such gadget is going to
> >> stay on this list forever so it's a bug.
> >
> > Ok, I can confirm this bug. But it is not the issue which I am
> > addressing with this patch. This is just about the
> > usb_gadget_probe_driver() return code (on which other functions,
> > paritcularly gadget configfs's|gadget_dev_desc_UDC_store|() rely).
> >
>
> This commit fixes both f1bddbb and 855ed04 just the bug is a little bit
> different but it's still a bug.
>
> >> Commit f1bddbb make this bug more visible (as it causes NULL pointer
> >> dereference) as gadget has not been added to the list of pending gadgets
> >> and we try to remove it from there.
> >>
> >> Summing up, in my personal opinion I think that you should add:
> >>
> >> Fixes: 855ed04 ("usb: gadget: udc-core: independent registration of
> >> gadgets and gadget drivers")
> >
> > As explained above, I think this would be wrong.
> >
>
> I understand that your target was to fix NULL ptr dereference but you
> can kill two birds with the same stone as you fix also the previous bug;).
>
> I suggested to place commit 855ed04 instead of f1bddbb because 855ed04
> appears earlier in a tree and even if someone doesn't have f1bddbb your
> patch is useful for him because it fix a bug introduced in that commit
> (gadget pending forever).
But it didn't introduce the bug which is fixed with this patch.
Best Regards,
Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb/gadget: GPF in usb_gadget_unregister_driver
Hi, > Hi! > > I've got the following error report while running the syzkaller fuzzer. > > On commit 3c49de52d5647cda8b42c4255cf8a29d1e22eff5 (Dec 2). > > general protection fault: [#1] SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > Modules linked in: > CPU: 0 PID: 10564 Comm: syz-executor0 Not tainted 4.9.0-rc7+ #4 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > task: 88006cd0db40 task.stack: 88006a05 > RIP: 0010:[] [] > __list_del_entry+0xce/0x280 lib/list_debug.c:57 > RSP: 0018:88006a056ea8 EFLAGS: 00010246 > RAX: dc00 RBX: 11000d40add6 RCX: 860ceac8 > RDX: RSI: 88006cd0e340 RDI: 860cead0 > RBP: 88006a056f38 R08: R09: > R10: 0006 R11: R12: > R13: R14: 860cea00 R15: 88006a056f10 > FS: () GS:88003ec0() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 2000d000 CR3: 05c1d000 CR4: 06f0 > Stack: > 859785e0 41b58ab3 8593717a 8201a9d0 > 11000d40ade0 88006cd0db40 817e5a3c 88006cd0e338 > 0a06 11000d40ade4 88006cd0e340 > Call Trace: > [] list_del+0xd/0x70 lib/list_debug.c:77 > [] usb_gadget_unregister_driver+0x120/0x240 > drivers/usb/gadget/udc/core.c:1365 > [] dev_release+0x80/0x160 > drivers/usb/gadget/legacy/inode.c:1187 > [] __fput+0x332/0x7f0 fs/file_table.c:208 > [] fput+0x15/0x20 fs/file_table.c:244 > [] task_work_run+0x19b/0x270 kernel/task_work.c:116 > [< inline >] exit_task_work include/linux/task_work.h:21 > [] do_exit+0x16aa/0x2530 kernel/exit.c:828 > [] do_group_exit+0x149/0x420 kernel/exit.c:932 > [] get_signal+0x76d/0x17b0 kernel/signal.c:2307 > [] do_signal+0xd2/0x2120 arch/x86/kernel/signal.c:807 > [] exit_to_usermode_loop+0x170/0x200 > arch/x86/entry/common.c:156 > [< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:190 > [] syscall_return_slowpath+0x3d3/0x420 > arch/x86/entry/common.c:259 > [] entry_SYSCALL_64_fastpath+0xc0/0xc2 > Code: c5 0f 84 e2 00 00 00 48 b8 00 02 00 00 00 00 ad de 49 39 c4 0f > 84 ec 00 00 00 4c 89 e2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> > 3c 02 00 0f 85 35 01 00 00 4d 8b 04 24 49 39 c8 0f 85 e1 00 > RIP [] __list_del_entry+0xce/0x280 lib/list_debug.c:57 > RSP > ---[ end trace 883f708e6720200f ]--- > Kernel panic - not syncing: Fatal exception > Dumping ftrace buffer: >(ftrace buffer empty) > Kernel Offset: disabled > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Looks similar to the oops which I had reported a few days ago. See mail "PROBLEM: Oops when unbinding an inactive gadget configfs configuration from UDC". Maybe you want to test my bugfix proposal "usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()". Regards, Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
Hi,
> Hi,
>
> Good catch but I have a few comments to commit message:
>
> On 12/01/2016 12:24 AM, Felix Hädicke wrote:
>> This fixes a regression which was introduced by commit f1bddbb, by
>> reverting a small fragment of commit 855ed04.
>>
> Not exactly. The regression has been introduced by commit 855ed04
> itself.This commit replaced previous construction similar what you sent
> now with simple if():
>
> @@ -535,11 +556,7 @@ int usb_gadget_probe_driver(struct
> usb_gadget_driver *driver)
> if (!ret)
> break;
> }
> - if (ret)
> - ret = -ENODEV;
> - else if (udc->driver)
> - ret = -EBUSY;
> - else
> + if (!ret && !udc->driver)
> goto found;
> } else {
> list_for_each_entry(udc, &udc_list, list) {
The regression bug I want to fix with this patch was introduced with
commit f1bddbb, not 855ed04. With 855ed04, the this if/else construct
was changed. But concerning the usb_gadget_probe_driver() return code,
this was ok. There was no code path were a value which was not meant to
be the function return code was accidentally returned. With commit
f1bddbb, and the introduction of a new code path for the flag
match_existing_only, this changed.
> After that commit, if UDC is busy, gadget is added to the list of
> pending gadgets. Unfortunately this list is not rescanned in
> usb_gadget_unregister_driver(). This means that such gadget is going to
> stay on this list forever so it's a bug.
Ok, I can confirm this bug. But it is not the issue which I am
addressing with this patch. This is just about the
usb_gadget_probe_driver() return code (on which other functions,
paritcularly gadget configfs's|gadget_dev_desc_UDC_store|() rely).
> Commit f1bddbb make this bug more visible (as it causes NULL pointer
> dereference) as gadget has not been added to the list of pending gadgets
> and we try to remove it from there.
>
> Summing up, in my personal opinion I think that you should add:
>
> Fixes: 855ed04 ("usb: gadget: udc-core: independent registration of
> gadgets and gadget drivers")
As explained above, I think this would be wrong.
> above your sign off and fix the commit message as this bug has been
> introduced before f1bddbb, just symptoms were a little bit different.
>
> After fixing this you may add:
>
> Tested-by: Krzysztof Opasiak
Ok.
>
> Best regards,
Best Regards,
Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: Oops when unbinding an inactive gadget configfs configuration from UDC
Hello, > I had expected that writing "dummy_udc.0" to > /sys/kernel/config/usb_gadget/gser/UDC would fail, because the UDC is > already bound to another gadget configuration > (/sys/kernel/config/usb_gadget/acm/UDC). However, this doesn't give me > an error, but the ACM configuration remains active. After investigating this further, I came to the conclusion that the false success report after trying to bind the second gadget configuration to the UDC is the actual cause for the oops. See bugfix propsal for a more detailed description: [PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver() Regards, Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: udc: core: fix return code of usb_gadget_probe_driver()
This fixes a regression which was introduced by commit f1bddbb, by reverting a small fragment of commit 855ed04. If the following conditions were met, usb_gadget_probe_driver() returned 0, although the call was unsuccessful: 1. A particular UDC was specified by thge gadget driver (using member "udc_name" of struct usb_gadget_driver). 2. The UDC with this name is available. 3. Another gadget driver is already bound to this gadget. 4. The gadget driver has the "match_existing_only" flag set. In this case, the return code variable "ret" is set to 0, the return code of a strcmp() call (to check for the second condition). This also fixes an oops which could occur in the following scenario: 1. Two usb gadget instances were configured using configfs. 2. The first gadget configuration was bound to a UDC (using the configfs attribute "UDC"). 3. It was tried to bind the second gadget configuration to the same UDC in the same way. This operation was then wrongly reported as being successful. 4. The second gadget configuration's "UDC" attribute is cleared, to unbind the (not really bound) second gadget configuration from the UDC. ] __list_del_entry+0x29/0xc0 PGD 41b4c5067 PUD 41a598067 PMD 0 Oops: [#1] SMP Modules linked in: cdc_acm usb_f_fs usb_f_serial usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core lpc_ich fat efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 task: 880419ce4040 task.stack: c90002ed4000 RIP: 0010:[] [] __list_del_entry+0x29/0xc0 RSP: 0018:c90002ed7d68 EFLAGS: 00010207 RAX: RBX: 88041787ec30 RCX: dead0200 RDX: RSI: 880417482002 RDI: 88041787ec30 RBP: c90002ed7d68 R08: R09: 0010 R10: R11: 880419ce4040 R12: 88041787eb68 R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001 FS: 7fe4e49b8700() GS:88042f34() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 00041b4c4000 CR4: 001406e0 Stack: c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0 c0ae22aa 88041787e800 88041787e800 c90002ed7dc0 c0d7a727 952273fa 88041aba5760 c90002ed7df8 Call Trace: [] list_del+0xd/0x30 [] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core] [] unregister_gadget+0x27/0x60 [libcomposite] [] ? mutex_lock+0x1a/0x30 [] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite] [] configfs_write_file+0xa0/0x100 [configfs] [] __vfs_write+0x37/0x160 [] ? __fd_install+0x30/0xd0 [] ? _raw_spin_unlock+0xe/0x10 [] vfs_write+0xb8/0x1b0 [] SyS_write+0x58/0xc0 [] ? __close_fd+0x94/0xc0 [] entry_SYSCALL_64_fastpath+0x1e/0xad Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89 e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b 02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08 RIP [] __list_del_entry+0x29/0xc0 RSP CR2: 0000 ---[ end trace 99fc090ab3ff6cbc ]--- Signed-off-by: Felix Hädicke --- drivers/usb/gadget/udc/core.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c index 9483489..0402177 100644 --- a/drivers/usb/gadget/udc/core.c +++ b/drivers/usb/gadget/udc/core.c @@ -1317,7 +1317,11 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver) if (!ret) break; } - if (!ret && !udc->driver) + if (ret) + ret = -ENODEV; + else if
PROBLEM: Oops when unbinding an inactive gadget configfs configuration from UDC
Hello, in 4.8.11 and 4.9-rc7, I get an Oops (see below), when I try to activate and afterward deactivate a second gadget configfs configuration, like this: mkdir /sys/kernel/config/usb_gadget/acm cd /sys/kernel/config/usb_gadget/acm echo 0x0200 > bcdUSB echo 0x0525 > idVendor echo 0xa4a7 > idProduct echo 0x03 > bDeviceClass mkdir functions/acm.1 mkdir configs/c.2 ln -s functions/acm.1/ configs/c.2 echo dummy_udc.0 > UDC mkdir /sys/kernel/config/usb_gadget/gser cd /sys/kernel/config/usb_gadget/gser echo 0x0525 > idVendor echo 0xa4a6 > idProduct echo 0xff > bDeviceClass mkdir functions/gser.1 mkdir configs/c.1 ln -s functions/gser.1/ configs/c.1 echo dummy_udc.0 > UDC echo > UDC I had expected that writing "dummy_udc.0" to /sys/kernel/config/usb_gadget/gser/UDC would fail, because the UDC is already bound to another gadget configuration (/sys/kernel/config/usb_gadget/acm/UDC). However, this doesn't give me an error, but the ACM configuration remains active. The same happens when using a real UDC (PLX3380 via driver net2280 in my case) instead of dummy_hcd. [ 600.276226] ] __list_del_entry+0x29/0xc0 [ 600.277450] PGD 41b4c5067 [ 600.277455] PUD 41a598067 [ 600.278063] PMD 0 [ 600.278680] Oops: [#1] SMP [ 600.279297] Modules linked in: cdc_acm usb_f_fs usb_f_serial usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core lpc_ich fat [ 600.280153] efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] [ 600.281688] CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1 [ 600.282424] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 [ 600.283163] task: 880419ce4040 task.stack: c90002ed4000 [ 600.283910] RIP: 0010:[] [] __list_del_entry+0x29/0xc0 [ 600.284668] RSP: 0018:c90002ed7d68 EFLAGS: 00010207 [ 600.285426] RAX: RBX: 88041787ec30 RCX: dead0200 [ 600.286192] RDX: RSI: 880417482002 RDI: 88041787ec30 [ 600.286955] RBP: c90002ed7d68 R08: R09: 0010 [ 600.287728] R10: R11: 880419ce4040 R12: 88041787eb68 [ 600.288513] R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001 [ 600.289300] FS: 7fe4e49b8700() GS:88042f34() knlGS: [ 600.290097] CS: 0010 DS: ES: CR0: 80050033 [ 600.290893] CR2: CR3: 00041b4c4000 CR4: 001406e0 [ 600.291696] Stack: [ 600.292504] c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0 [ 600.293327] c0ae22aa 88041787e800 88041787e800 c90002ed7dc0 [ 600.294153] c0d7a727 952273fa 88041aba5760 c90002ed7df8 [ 600.294985] Call Trace: [ 600.295809] [] list_del+0xd/0x30 [ 600.296653] [] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core] [ 600.297496] [] unregister_gadget+0x27/0x60 [libcomposite] [ 600.298342] [] ? mutex_lock+0x1a/0x30 [ 600.299188] [] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite] [ 600.300046] [] configfs_write_file+0xa0/0x100 [configfs] [ 600.300904] [] __vfs_write+0x37/0x160 [ 600.301761] [] ? __fd_install+0x30/0xd0 [ 600.302627] [] ? _raw_spin_unlock+0xe/0x10 [ 600.303494] [] vfs_write+0xb8/0x1b0 [ 600.304364] [] SyS_write+0x58/0xc0 [ 600.305228] [] ? __close_fd+0x94/0xc0 [ 600.306103] [] entry_SYSCALL_64_fastpath+0x1e/0xad [ 600.306980] Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89 e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b 02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08 [ 600.308031] RIP [] __list_del_entry+0x29/0xc0 [ 600.308960] RSP [ 600.309880] CR2: [ 600.310845] ---[ end trace 99fc090ab3ff6cbc ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a m
PROBLEM: Oops when
Hello, in 4.8.11 and 4.9-rc7, I get an Oops (see below), when I try to activate and afterward deactivate a second gadget configfs configuration, like this: mkdir /sys/kernel/config/usb_gadget/acm cd /sys/kernel/config/usb_gadget/acm echo 0x0200 > bcdUSB echo 0x0525 > idVendor echo 0xa4a7 > idProduct echo 0x03 > bDeviceClass mkdir functions/acm.1 mkdir configs/c.2 ln -s functions/acm.1/ configs/c.2 echo dummy_udc.0 > UDC mkdir /sys/kernel/config/usb_gadget/gser cd /sys/kernel/config/usb_gadget/gser echo 0x0525 > idVendor echo 0xa4a6 > idProduct echo 0xff > bDeviceClass mkdir functions/gser.1 mkdir configs/c.1 ln -s functions/gser.1/ configs/c.1 echo dummy_udc.0 > UDC echo > UDC I had expected that writing "dummy_udc.0" to /sys/kernel/config/usb_gadget/gser/UDC would fail, because the UDC is already bound to another gadget configuration (/sys/kernel/config/usb_gadget/acm/UDC). However, this doesn't give me an error, but the ACM configuration remains active. The same happens when using a real UDC (PLX3380 via driver net2280 in my case) instead of dummy_hcd. [ 600.276226] ] __list_del_entry+0x29/0xc0 [ 600.277450] PGD 41b4c5067 [ 600.277455] PUD 41a598067 [ 600.278063] PMD 0 [ 600.278680] Oops: [#1] SMP [ 600.279297] Modules linked in: cdc_acm usb_f_fs usb_f_serial usb_f_acm u_serial libcomposite configfs dummy_hcd bnep intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm snd_hda_codec_hdmi irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd snd_hda_codec_realtek snd_hda_codec_generic serio_raw uvcvideo videobuf2_vmalloc btusb snd_usb_audio snd_hda_intel videobuf2_memops btrtl snd_hda_codec snd_hda_core snd_usbmidi_lib btbcm videobuf2_v4l2 btintel snd_hwdep videobuf2_core snd_seq_midi bluetooth snd_seq_midi_event videodev xpad efi_pstore snd_pcm_oss rfkill joydev media crc16 ff_memless snd_mixer_oss snd_rawmidi nls_ascii snd_pcm snd_seq snd_seq_device nls_cp437 mei_me snd_timer vfat sg udc_core lpc_ich fat [ 600.280153] efivars mfd_core mei snd soundcore battery nuvoton_cir rc_core evdev intel_smartconnect ie31200_edac edac_core shpchp tpm_tis tpm_tis_core tpm parport_pc ppdev lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom sd_mod ahci libahci nouveau i915 crc32c_intel i2c_algo_bit psmouse ttm xhci_pci libata scsi_mod ehci_pci drm_kms_helper xhci_hcd ehci_hcd r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] [ 600.281688] CPU: 5 PID: 829 Comm: bash Not tainted 4.9.0-rc7 #1 [ 600.282424] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 [ 600.283163] task: 880419ce4040 task.stack: c90002ed4000 [ 600.283910] RIP: 0010:[] [] __list_del_entry+0x29/0xc0 [ 600.284668] RSP: 0018:c90002ed7d68 EFLAGS: 00010207 [ 600.285426] RAX: RBX: 88041787ec30 RCX: dead0200 [ 600.286192] RDX: RSI: 880417482002 RDI: 88041787ec30 [ 600.286955] RBP: c90002ed7d68 R08: R09: 0010 [ 600.287728] R10: R11: 880419ce4040 R12: 88041787eb68 [ 600.288513] R13: 88041787eaa8 R14: 88041560a2c0 R15: 0001 [ 600.289300] FS: 7fe4e49b8700() GS:88042f34() knlGS: [ 600.290097] CS: 0010 DS: ES: CR0: 80050033 [ 600.290893] CR2: CR3: 00041b4c4000 CR4: 001406e0 [ 600.291696] Stack: [ 600.292504] c90002ed7d80 94f5e68d c0ae5ef0 c90002ed7da0 [ 600.293327] c0ae22aa 88041787e800 88041787e800 c90002ed7dc0 [ 600.294153] c0d7a727 952273fa 88041aba5760 c90002ed7df8 [ 600.294985] Call Trace: [ 600.295809] [] list_del+0xd/0x30 [ 600.296653] [] usb_gadget_unregister_driver+0xaa/0xc0 [udc_core] [ 600.297496] [] unregister_gadget+0x27/0x60 [libcomposite] [ 600.298342] [] ? mutex_lock+0x1a/0x30 [ 600.299188] [] gadget_dev_desc_UDC_store+0x88/0xe0 [libcomposite] [ 600.300046] [] configfs_write_file+0xa0/0x100 [configfs] [ 600.300904] [] __vfs_write+0x37/0x160 [ 600.301761] [] ? __fd_install+0x30/0xd0 [ 600.302627] [] ? _raw_spin_unlock+0xe/0x10 [ 600.303494] [] vfs_write+0xb8/0x1b0 [ 600.304364] [] SyS_write+0x58/0xc0 [ 600.305228] [] ? __close_fd+0x94/0xc0 [ 600.306103] [] entry_SYSCALL_64_fastpath+0x1e/0xad [ 600.306980] Code: 66 90 55 48 8b 07 48 b9 00 01 00 00 00 00 ad de 48 8b 57 08 48 89 e5 48 39 c8 74 29 48 b9 00 02 00 00 00 00 ad de 48 39 ca 74 3a <4c> 8b 02 4c 39 c7 75 52 4c 8b 40 08 4c 39 c7 75 66 48 89 50 08 [ 600.308031] RIP [] __list_del_entry+0x29/0xc0 [ 600.308960] RSP [ 600.309880] CR2: [ 600.310845] ---[ end trace 99fc090ab3ff6cbc ]--- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a me
Re: [PATCH] usb: gadget: f_fs: fix wrong parenthesis in ffs_func_req_match()
Hello,
> Properly check the return code of ffs_func_revmap_intf() and
> ffs_func_revmap_ep() for a non-negative value.
>
> Instead of checking the return code, the comparison was performed for the last
> parameter of the function calls, because of wrong parenthesis.
>
> This also fixes the following static checker warning:
> drivers/usb/gadget/function/f_fs.c:3152 ffs_func_req_match()
> warn: always true condition '(((creq->wIndex)) >= 0) => (0-u16max >= 0)'
>
> Reported-by: Dan Carpenter
> Signed-off-by: Felix Hädicke
> ---
> drivers/usb/gadget/function/f_fs.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index 43aeed6..fcfdc6a 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -3028,11 +3028,11 @@ static bool ffs_func_req_match(struct usb_function *f,
>
> switch (creq->bRequestType & USB_RECIP_MASK) {
> case USB_RECIP_INTERFACE:
> - return ffs_func_revmap_intf(func,
> - le16_to_cpu(creq->wIndex) >= 0);
> + return (ffs_func_revmap_intf(func,
> + le16_to_cpu(creq->wIndex)) >= 0);
> case USB_RECIP_ENDPOINT:
> - return ffs_func_revmap_ep(func,
> - le16_to_cpu(creq->wIndex) >= 0);
> + return (ffs_func_revmap_ep(func,
> +le16_to_cpu(creq->wIndex)) >= 0);
> default:
> return (bool) (func->ffs->user_flags &
> FUNCTIONFS_ALL_CTRL_RECIP);
is there a chance to get this fix into 4.9?
Regards,
Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: serial: fix possible Oops caused by calling kthread_stop(NULL)
Add check for NULL before calling kthread_stop(). There were cases in which gserial_console_exit() was called, but the console thread was not started. This resulted in an invalid kthread_stop(NULL) call. Without this, the following Oops may occur: BUG: unable to handle kernel NULL pointer dereference at 0018 IP: [] kthread_stop+0x16/0x110 ... CPU: 2 PID: 853 Comm: rmmod Not tainted 4.9.0-rc5 #3 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 task: 880419f6a100 task.stack: c90002e8c000 RIP: 0010:[] [] kthread_stop+0x16/0x110 RSP: 0018:c90002e8fdb0 EFLAGS: 00010286 RAX: 0001 RBX: RCX: RDX: 0001 RSI: 0246 RDI: RBP: c90002e8fdc8 R08: R09: 0001 R10: 019d R11: 001f R12: R13: 88041b8d8400 R14: 0001 R15: 55fd59f5a1e0 FS: 7f82500be700() GS:88042f28() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 0018 CR3: 00041bee2000 CR4: 001406e0 Stack: c0b8e720 88041b8d8400 c90002e8fdf0 c0b8bb52 88041a106300 0001 880419fc2ea8 c90002e8fe08 c0aed749 c0aef600 c90002e8fe20 Call Trace: [] gserial_free_line+0x72/0xb0 [u_serial] [] acm_free_instance+0x19/0x30 [usb_f_acm] [] usb_put_function_instance+0x20/0x30 [libcomposite] [] gs_unbind+0x3b/0x70 [g_serial] [] __composite_unbind+0x61/0xb0 [libcomposite] [] composite_unbind+0x13/0x20 [libcomposite] [] usb_gadget_remove_driver+0x3d/0x90 [udc_core] [] usb_gadget_unregister_driver+0x6e/0xc0 [udc_core] [] usb_composite_unregister+0x12/0x20 [libcomposite] [] cleanup+0x10/0xda8 [g_serial] [] SyS_delete_module+0x192/0x270 [] ? exit_to_usermode_loop+0x90/0xb0 [] entry_SYSCALL_64_fastpath+0x1e/0xad Code: 89 c6 e8 6e ff ff ff 48 89 df e8 06 bd fd ff 5b 5d c3 0f 1f 00 0f 1f 44 00 00 55 48 89 e5 41 55 41 54 49 89 fc 53 0f 1f 44 00 00 41 ff 44 24 18 4c 89 e7 e8 bc f1 ff ff 48 85 c0 48 89 c3 74 RIP [] kthread_stop+0x16/0x110 RSP CR2: 0018 ---[ end trace 5b3336a407e1698c ]--- Signed-off-by: Felix Hädicke Tested-by: Peter Chen --- Changes in v2: - Add Tested-by: Peter Chen - Add oops dump drivers/usb/gadget/function/u_serial.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 5fedead..f58c775 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1256,7 +1256,8 @@ static void gserial_console_exit(void) struct gscons_info *info = &gscons_info; unregister_console(&gserial_cons); - kthread_stop(info->console_thread); + if (info->console_thread != NULL) + kthread_stop(info->console_thread); gs_buf_free(&info->con_buf); } -- 2.10.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: serial: zero-initialize struct variable gscons_info
ok, thanks for the hint. I dind't know this.
Just ignore this patch.
Am 16.11.2016 um 22:42 schrieb Sergei Shtylyov:
> Hello.
>
> On 11/16/2016 11:46 PM, Felix Hädicke wrote:
>
>> There can be cases when members of the gscons_info struct are used
>> uninitialized, e. g. in, gserial_console_exit(), if gs_console_setup()
>> was not called before.
>>
>> Signed-off-by: Felix Hädicke
>> ---
>> drivers/usb/gadget/function/u_serial.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/function/u_serial.c
>> b/drivers/usb/gadget/function/u_serial.c
>> index e0cd1e4..5fedead 100644
>> --- a/drivers/usb/gadget/function/u_serial.c
>> +++ b/drivers/usb/gadget/function/u_serial.c
>> @@ -1043,7 +1043,7 @@ static struct tty_driver *gs_tty_driver;
>>
>> #ifdef CONFIG_U_SERIAL_CONSOLE
>>
>> -static struct gscons_info gscons_info;
>> +static struct gscons_info gscons_info = {};
>
>That shouldn't change anything -- static variables are always zeroed.
>
> [...]
>
> MBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: gadget: serial: zero-initialize struct variable gscons_info
There can be cases when members of the gscons_info struct are used
uninitialized, e. g. in, gserial_console_exit(), if gs_console_setup()
was not called before.
Signed-off-by: Felix Hädicke
---
drivers/usb/gadget/function/u_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index e0cd1e4..5fedead 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1043,7 +1043,7 @@ static struct tty_driver *gs_tty_driver;
#ifdef CONFIG_U_SERIAL_CONSOLE
-static struct gscons_info gscons_info;
+static struct gscons_info gscons_info = {};
static struct console gserial_cons;
static struct usb_request *gs_request_new(struct usb_ep *ep)
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: gadget: serial: fix possible Oops caused by calling kthread_stop(NULL)
Add check for NULL before calling kthread_stop(). There were cases in which gserial_console_exit() was called, but the console thread was not started. This resulted in an invalid kthread_stop(NULL) call. Signed-off-by: Felix Hädicke --- drivers/usb/gadget/function/u_serial.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 5fedead..f58c775 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1256,7 +1256,8 @@ static void gserial_console_exit(void) struct gscons_info *info = &gscons_info; unregister_console(&gserial_cons); - kthread_stop(info->console_thread); + if (info->console_thread != NULL) + kthread_stop(info->console_thread); gs_buf_free(&info->con_buf); } -- 2.10.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PROBLEM: Oops when deactivating gadget serial driver
Hi, For me, it is always reproducible when CONFIG_U_SERIAL_CONSOLE is enabled. But I think that GDB is misleading, it has probably nothing to do with gs_buf_free(). For me, it looks like the real problem is that in gserial_console_exit(), kthread_stop() is called on the (in my case) uninitialised variable gscons_info.console_thread. I am going to propose a fix. Regards Felix > Hello, > > In 4.8.6 and 4.9-rc5, the gadget serial driver crashes during > deinitialisation when compiled with CONFIG_U_SERIAL_CONSOLE. > > Steps to reproduce: > modprobe g-serial > rmmod g-serial > > The problem also occurs when using configfs, when the UDC is unbound. > > I does not make a difference if I use my PLX3380 (driver net2280) as > UDC, or dummy-hcd. > > > excerpt from dmesg output (complete output see below): > > [ 75.705246] BUG: unable to handle kernel > [ 75.751165] NULL pointer dereference > [ 75.791826] at 0018 > [ 75.814807] IP: [] kthread_stop+0x16/0x110 > [ 75.882611] PGD 0 > > [ 75.922339] Oops: 0002 [#1] SMP > [ 75.959880] Modules linked in: cdc_acm usb_f_acm u_serial g_serial(-) > libcomposite configfs dummy_hcd bnep nls_ascii nls_cp437 vfat fat > intel_rapl x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass > crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 > lrw gf128mul glue_helper ablk_helper cryptd uvcvideo videobuf2_vmalloc > videobuf2_memops videobuf2_v4l2 videobuf2_core videodev xpad ff_memless > media joydev snd_hda_codec_hdmi snd_usb_audio snd_usbmidi_lib > snd_hda_codec_realtek btusb btrtl efi_pstore btbcm snd_hda_codec_generic > btintel bluetooth serio_raw rfkill crc16 efivars snd_hda_intel sg > snd_hda_codec snd_hda_core snd_hwdep snd_seq_midi snd_seq_midi_event > snd_pcm_oss snd_rawmidi snd_mixer_oss udc_core snd_pcm snd_seq > snd_seq_device lpc_ich snd_timer mfd_core snd soundcore battery > [ 76.816163] nuvoton_cir rc_core mei_me mei evdev intel_smartconnect > shpchp ie31200_edac tpm_tis tpm_tis_core tpm edac_core parport_pc ppdev > lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp > hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom > sd_mod nouveau ahci libahci i915 crc32c_intel libata ttm i2c_algo_bit > ehci_pci xhci_pci psmouse xhci_hcd drm_kms_helper ehci_hcd scsi_mod > r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280] > [ 77.316999] CPU: 2 PID: 853 Comm: rmmod Not tainted 4.9.0-rc5 #3 > [ 77.388856] Hardware name: To Be Filled By O.E.M. To Be Filled By > O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013 > [ 77.506474] task: 880419f6a100 task.stack: c90002e8c000 > [ 77.577292] RIP: 0010:[] [] > kthread_stop+0x16/0x110 > [ 77.674214] RSP: 0018:c90002e8fdb0 EFLAGS: 00010286 > [ 77.737754] RAX: 0001 RBX: RCX: > > [ 77.823133] RDX: 0001 RSI: 0246 RDI: > > [ 77.908513] RBP: c90002e8fdc8 R08: R09: > 0001 > [ 77.993892] R10: 019d R11: 001f R12: > > [ 78.079271] R13: 88041b8d8400 R14: 0001 R15: > 55fd59f5a1e0 > [ 78.164649] FS: 7f82500be700() GS:88042f28() > knlGS: > [ 78.261467] CS: 0010 DS: ES: CR0: 80050033 > [ 78.330206] CR2: 0018 CR3: 00041bee2000 CR4: > 001406e0 > [ 78.415586] Stack: > [ 78.439609] c0b8e720 88041b8d8400 > c90002e8fdf0 > [ 78.528522] c0b8bb52 88041a106300 0001 > 880419fc2ea8 > [ 78.617436] c90002e8fe08 c0aed749 c0aef600 > c90002e8fe20 > [ 78.706350] Call Trace: > [ 78.735579] [] gserial_free_line+0x72/0xb0 [u_serial] > [ 78.815758] [] acm_free_instance+0x19/0x30 [usb_f_acm] > [ 78.896978] [] usb_put_function_instance+0x20/0x30 > [libcomposite] > [ 78.989634] [] gs_unbind+0x3b/0x70 [g_serial] > [ 79.061493] [] __composite_unbind+0x61/0xb0 > [libcomposite] > [ 79.146872] [] composite_unbind+0x13/0x20 > [libcomposite] > [ 79.230172] [] usb_gadget_remove_driver+0x3d/0x90 > [udc_core] > [ 79.317632] [] > usb_gadget_unregister_driver+0x6e/0xc0 [udc_core] > [ 79.409248] [] usb_composite_unregister+0x12/0x20 > [libcomposite] > [ 79.500866] [] cleanup+0x10/0xda8 [g_serial] > [ 79.571685] [] SyS_delete_module+0x192/0x270 > [ 79.642508] [] ? exit_to_usermode_loop+0x90/0xb0 > [ 79.717486] [] entry_SYSCALL_64_fastpath+0x1e/0xad > [ 79.794543] Code: 89 c6 e8 6e ff ff ff 48 89 df e8 06 bd fd ff 5b 5d > c3 0f 1f 00 0f 1f 44 00 00 55 48 89 e5 41 55 41 54 49 89 fc 53 0f 1f 44 > 00 00 41 ff 44 24 18 4c 89 e7 e8 bc f1 ff ff 48 85 c0 48 89 c3 74 > [ 80.027071] RIP [] kthread_stop+0x16/0x110 > [ 80.095915] RSP > [ 80.137617] CR2: 0018 > [ 80.177270] ---[ end trace 5b3336a407e1698c ]--- > > > (gdb) list *(gserial_free_line+0x72) > 0x1b
PROBLEM: Oops when deactivating gadget serial driver
Hello,
In 4.8.6 and 4.9-rc5, the gadget serial driver crashes during
deinitialisation when compiled with CONFIG_U_SERIAL_CONSOLE.
Steps to reproduce:
modprobe g-serial
rmmod g-serial
The problem also occurs when using configfs, when the UDC is unbound.
I does not make a difference if I use my PLX3380 (driver net2280) as
UDC, or dummy-hcd.
excerpt from dmesg output (complete output see below):
[ 75.705246] BUG: unable to handle kernel
[ 75.751165] NULL pointer dereference
[ 75.791826] at 0018
[ 75.814807] IP: [] kthread_stop+0x16/0x110
[ 75.882611] PGD 0
[ 75.922339] Oops: 0002 [#1] SMP
[ 75.959880] Modules linked in: cdc_acm usb_f_acm u_serial g_serial(-)
libcomposite configfs dummy_hcd bnep nls_ascii nls_cp437 vfat fat
intel_rapl x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64
lrw gf128mul glue_helper ablk_helper cryptd uvcvideo videobuf2_vmalloc
videobuf2_memops videobuf2_v4l2 videobuf2_core videodev xpad ff_memless
media joydev snd_hda_codec_hdmi snd_usb_audio snd_usbmidi_lib
snd_hda_codec_realtek btusb btrtl efi_pstore btbcm snd_hda_codec_generic
btintel bluetooth serio_raw rfkill crc16 efivars snd_hda_intel sg
snd_hda_codec snd_hda_core snd_hwdep snd_seq_midi snd_seq_midi_event
snd_pcm_oss snd_rawmidi snd_mixer_oss udc_core snd_pcm snd_seq
snd_seq_device lpc_ich snd_timer mfd_core snd soundcore battery
[ 76.816163] nuvoton_cir rc_core mei_me mei evdev intel_smartconnect
shpchp ie31200_edac tpm_tis tpm_tis_core tpm edac_core parport_pc ppdev
lp parport efivarfs autofs4 btrfs xor raid6_pq hid_logitech_hidpp
hid_logitech_dj hid_generic usbhid hid uas usb_storage sr_mod cdrom
sd_mod nouveau ahci libahci i915 crc32c_intel libata ttm i2c_algo_bit
ehci_pci xhci_pci psmouse xhci_hcd drm_kms_helper ehci_hcd scsi_mod
r8169 mii usbcore drm nvme nvme_core fjes button [last unloaded: net2280]
[ 77.316999] CPU: 2 PID: 853 Comm: rmmod Not tainted 4.9.0-rc5 #3
[ 77.388856] Hardware name: To Be Filled By O.E.M. To Be Filled By
O.E.M./Z77 Extreme3, BIOS P1.50 07/11/2013
[ 77.506474] task: 880419f6a100 task.stack: c90002e8c000
[ 77.577292] RIP: 0010:[] []
kthread_stop+0x16/0x110
[ 77.674214] RSP: 0018:c90002e8fdb0 EFLAGS: 00010286
[ 77.737754] RAX: 0001 RBX: RCX:
[ 77.823133] RDX: 0001 RSI: 0246 RDI:
[ 77.908513] RBP: c90002e8fdc8 R08: R09:
0001
[ 77.993892] R10: 019d R11: 001f R12:
[ 78.079271] R13: 88041b8d8400 R14: 0001 R15:
55fd59f5a1e0
[ 78.164649] FS: 7f82500be700() GS:88042f28()
knlGS:
[ 78.261467] CS: 0010 DS: ES: CR0: 80050033
[ 78.330206] CR2: 0018 CR3: 00041bee2000 CR4:
001406e0
[ 78.415586] Stack:
[ 78.439609] c0b8e720 88041b8d8400
c90002e8fdf0
[ 78.528522] c0b8bb52 88041a106300 0001
880419fc2ea8
[ 78.617436] c90002e8fe08 c0aed749 c0aef600
c90002e8fe20
[ 78.706350] Call Trace:
[ 78.735579] [] gserial_free_line+0x72/0xb0 [u_serial]
[ 78.815758] [] acm_free_instance+0x19/0x30 [usb_f_acm]
[ 78.896978] [] usb_put_function_instance+0x20/0x30
[libcomposite]
[ 78.989634] [] gs_unbind+0x3b/0x70 [g_serial]
[ 79.061493] [] __composite_unbind+0x61/0xb0
[libcomposite]
[ 79.146872] [] composite_unbind+0x13/0x20
[libcomposite]
[ 79.230172] [] usb_gadget_remove_driver+0x3d/0x90
[udc_core]
[ 79.317632] []
usb_gadget_unregister_driver+0x6e/0xc0 [udc_core]
[ 79.409248] [] usb_composite_unregister+0x12/0x20
[libcomposite]
[ 79.500866] [] cleanup+0x10/0xda8 [g_serial]
[ 79.571685] [] SyS_delete_module+0x192/0x270
[ 79.642508] [] ? exit_to_usermode_loop+0x90/0xb0
[ 79.717486] [] entry_SYSCALL_64_fastpath+0x1e/0xad
[ 79.794543] Code: 89 c6 e8 6e ff ff ff 48 89 df e8 06 bd fd ff 5b 5d
c3 0f 1f 00 0f 1f 44 00 00 55 48 89 e5 41 55 41 54 49 89 fc 53 0f 1f 44
00 00 41 ff 44 24 18 4c 89 e7 e8 bc f1 ff ff 48 85 c0 48 89 c3 74
[ 80.027071] RIP [] kthread_stop+0x16/0x110
[ 80.095915] RSP
[ 80.137617] CR2: 0018
[ 80.177270] ---[ end trace 5b3336a407e1698c ]---
(gdb) list *(gserial_free_line+0x72)
0x1b52 is in gserial_free_line (drivers/usb/gadget/function/u_serial.c:187).
182 *
183 * Free the buffer and all associated memory.
184 */
185 static void gs_buf_free(struct gs_buf *gb)
186 {
187 kfree(gb->buf_buf);
188 gb->buf_buf = NULL;
189 }
190
191 /*
Complete dmesg output:
[0.00] Linux version 4.9.0-rc5 (root@han) (gcc version 5.3.1
20160413 (Ubuntu 5.3.1-14ubuntu2) ) #3 SMP Tue Nov 15 22:38:23 UTC 2016
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.9.0-rc5
root=UUID=97b8d297-4ca3-40ad-b9c3-ba803
[PATCH] usb: gadget: f_fs: fix wrong parenthesis in ffs_func_req_match()
Properly check the return code of ffs_func_revmap_intf() and
ffs_func_revmap_ep() for a non-negative value.
Instead of checking the return code, the comparison was performed for the last
parameter of the function calls, because of wrong parenthesis.
This also fixes the following static checker warning:
drivers/usb/gadget/function/f_fs.c:3152 ffs_func_req_match()
warn: always true condition '(((creq->wIndex)) >= 0) => (0-u16max >= 0)'
Reported-by: Dan Carpenter
Signed-off-by: Felix Hädicke
---
drivers/usb/gadget/function/f_fs.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 43aeed6..fcfdc6a 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -3028,11 +3028,11 @@ static bool ffs_func_req_match(struct usb_function *f,
switch (creq->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
- return ffs_func_revmap_intf(func,
- le16_to_cpu(creq->wIndex) >= 0);
+ return (ffs_func_revmap_intf(func,
+le16_to_cpu(creq->wIndex)) >= 0);
case USB_RECIP_ENDPOINT:
- return ffs_func_revmap_ep(func,
- le16_to_cpu(creq->wIndex) >= 0);
+ return (ffs_func_revmap_ep(func,
+ le16_to_cpu(creq->wIndex)) >= 0);
default:
return (bool) (func->ffs->user_flags &
FUNCTIONFS_ALL_CTRL_RECIP);
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [bug report] usb: gadget: f_fs: handle control requests not directed to interface or endpoint
Hello Dan Carpenter,
thanks for this. It is indeed a parentheses error. I am currently
testing a patch to correct this.
Regards,
Felix
> Hello Felix Hädicke,
>
> The patch 54dfce6d07b0: "usb: gadget: f_fs: handle control requests
> not directed to interface or endpoint" from Jun 22, 2016, leads to
> the following static checker warning:
>
> drivers/usb/gadget/function/f_fs.c:3152 ffs_func_req_match()
> warn: always true condition '(((creq->wIndex)) >= 0) => (0-u16max >= 0)'
>
> drivers/usb/gadget/function/f_fs.c
> 3140 static bool ffs_func_req_match(struct usb_function *f,
> 3141 const struct usb_ctrlrequest *creq,
> 3142 bool config0)
> 3143 {
> 3144 struct ffs_function *func = ffs_func_from_usb(f);
> 3145
> 3146 if (config0 && !(func->ffs->user_flags &
> FUNCTIONFS_CONFIG0_SETUP))
> 3147 return false;
> 3148
> 3149 switch (creq->bRequestType & USB_RECIP_MASK) {
> 3150 case USB_RECIP_INTERFACE:
> 3151 return ffs_func_revmap_intf(func,
> 3152 le16_to_cpu(creq->wIndex)
> >= 0);
>
> ^
> 3153 case USB_RECIP_ENDPOINT:
> 3154 return ffs_func_revmap_ep(func,
> 3155le16_to_cpu(creq->wIndex)
> >= 0);
>
> ^^
> This doesn't work, but it's not even clear to me what we are trying to
> do here.
>
> 3156 default:
> 3157 return (bool) (func->ffs->user_flags &
> 3158 FUNCTIONFS_ALL_CTRL_RECIP);
> 3159 }
> 3160 }
>
> regards,
> dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 1/3] usb: gadget: f_fs: handle control requests not directed to interface or endpoint
> This is actually not for a f_accessory replacement. The control requests are > sent when the device is in any other gadget mode, for sending metadata and to > trigger switching to Open Accessory Gadget Mode. > > The Accessory mode itself can be handled very easily with f_serial (or a very > simple FunctionFS driver). So I wonder why f_accessory exists. ok, I see that f_accessory can handle much more (HID and sound). But all of this could be handled in a FunctionFS driver as well. Regards Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 1/3] usb: gadget: f_fs: handle control requests not directed to interface or endpoint
Hello, > > I am developing a driver, which I would like to implement using > > FunctionFS, for using the Android Open Accessory protocol on > > non-Android devices. These accessories send some non-standard control > > requests which are described here: > > https://source.android.com/devices/accessories/aoa.html > > oh cool. I was thinking about Android Accessory a couple weeks ago. I'm > not sure yet if functionfs is the best way to go here. Maybe we should > actually have a proper function (f_android_accessory.c??) so it's easier > to handle this. > > Also, maybe we should add some Android folks to the loop here. Adding > android's accessory function author here. > > Mike, I'm looking at [1]. Do you have any interest in getting that > upstream? Also, taking the opportunity to raise another question: I > noticed android's MTP is pretty darn slow, I'm assuming the limitation > is functionfs itself. Do you wanna see how we can make that faster? This is actually not for a f_accessory replacement. The control requests are sent when the device is in any other gadget mode, for sending metadata and to trigger switching to Open Accessory Gadget Mode. The Accessory mode itself can be handled very easily with f_serial (or a very simple FunctionFS driver). So I wonder why f_accessory exists. Regards Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 2/3] usb: gadget: composite: ability for USB functions to process control requests in config 0
Hello, > Hi, > > Felix Hädicke writes: > > It can sometimes be necessary for gadget drivers to process non-standard > > control requests, which host devices can send without having sent > > USB_REQ_SET_CONFIGURATION. > > got an example? There are Android Open Accessory devices which do so. The IOIO microcontroller board does it https://github.com/ytai/ioio/wiki/IOIO-Over-OpenAccessory > > -- > balbi > Regards Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 1/3] usb: gadget: f_fs: handle control requests not directed to interface or endpoint
Hello, > Hi, > > Felix Hädicke writes: > > Introduces a new FunctionFS descriptor flag named > > FUNCTIONFS_ALL_CTRL_RECIP. When this flag is enabled, control requests, > > which are not explicitly directed to an interface or endpoint, can be > > handled. > > > > This allows FunctionFS userspace drivers to process non-standard > > control requests. > > > > Signed-off-by: Felix Hädicke > > can you describe a situation where this is needed? That would be > valuable for commit log I am developing a driver, which I would like to implement using FunctionFS, for using the Android Open Accessory protocol on non-Android devices. These accessories send some non-standard control requests which are described here: https://source.android.com/devices/accessories/aoa.html > > -- > balbi > Regards Felix -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] usb: gadget: composite: ability for USB functions to process control requests in config 0
It can sometimes be necessary for gadget drivers to process non-standard
control requests, which host devices can send without having sent
USB_REQ_SET_CONFIGURATION.
Therefore, the req_match() usb_function method is enhanced with the new
parameter "config0". When a USB configuration is active, this parameter
is false. When a non-core control request is processed in
composite_setup(), without an active configuration, req_match() of the
USB functions of all available configurations which implement this
function, is called with config0=true. Then the control request gets
processed by the first usb_function instance whose req_match() returns
true.
Signed-off-by: Felix Hädicke
---
drivers/usb/gadget/composite.c | 16 ++--
drivers/usb/gadget/function/f_fs.c | 9 +++--
drivers/usb/gadget/function/f_printer.c | 6 +-
include/linux/usb/composite.h | 3 ++-
4 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 524e233..81ed5a8 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1872,17 +1872,21 @@ unknown:
/* functions always handle their interfaces and endpoints...
* punt other recipients (other, WUSB, ...) to the current
* configuration code.
-*
-* REVISIT it could make sense to let the composite device
-* take such requests too, if that's ever needed: to work
-* in config 0, etc.
*/
if (cdev->config) {
list_for_each_entry(f, &cdev->config->functions, list)
- if (f->req_match && f->req_match(f, ctrl))
+ if (f->req_match &&
+ f->req_match(f, ctrl, false))
goto try_fun_setup;
- f = NULL;
+ } else {
+ struct usb_configuration *c;
+ list_for_each_entry(c, &cdev->configs, list)
+ list_for_each_entry(f, &c->functions, list)
+ if (f->req_match &&
+ f->req_match(f, ctrl, true))
+ goto try_fun_setup;
}
+ f = NULL;
switch (ctrl->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 9eb4003..4cf6efc 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -99,7 +99,8 @@ static void ffs_func_disable(struct usb_function *);
static int ffs_func_setup(struct usb_function *,
const struct usb_ctrlrequest *);
static bool ffs_func_req_match(struct usb_function *,
- const struct usb_ctrlrequest *);
+ const struct usb_ctrlrequest *,
+ bool config0);
static void ffs_func_suspend(struct usb_function *);
static void ffs_func_resume(struct usb_function *);
@@ -3016,10 +3017,14 @@ static int ffs_func_setup(struct usb_function *f,
}
static bool ffs_func_req_match(struct usb_function *f,
- const struct usb_ctrlrequest *creq)
+ const struct usb_ctrlrequest *creq,
+ bool config0)
{
struct ffs_function *func = ffs_func_from_usb(f);
+ if (config0)
+ return false;
+
switch (creq->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
return ffs_func_revmap_intf(func,
diff --git a/drivers/usb/gadget/function/f_printer.c
b/drivers/usb/gadget/function/f_printer.c
index c45104e..aebffc6 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -897,13 +897,17 @@ static void printer_soft_reset(struct printer_dev *dev)
/*-*/
static bool gprinter_req_match(struct usb_function *f,
- const struct usb_ctrlrequest *ctrl)
+ const struct usb_ctrlrequest *ctrl,
+ bool config0)
{
struct printer_dev *dev = func_to_printer(f);
u16 w_index = le16_to_cpu(ctrl->wIndex);
u16 w_value = le16_to_cpu(ctrl->wValue);
u16 w_length = le16_to_cpu(ctrl->wLength);
+ if (config0)
+ return false;
+
if ((ctrl->bRequestType & USB_RECIP_MASK) != USB_RECIP_INTERFACE ||
(c
[PATCH 3/3] usb: gadget: f_fs: handle control requests in config 0
Introduces a new FunctionFS descriptor flag named
FUNCTIONFS_CONFIG0_SETUP.
When this flag is enabled, FunctionFS userspace drivers can process
non-standard control requests in configuration 0.
Signed-off-by: Felix Hädicke
---
drivers/usb/gadget/function/f_fs.c | 5 +++--
include/uapi/linux/usb/functionfs.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 4cf6efc..43aeed6 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2126,7 +2126,8 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
FUNCTIONFS_HAS_MS_OS_DESC |
FUNCTIONFS_VIRTUAL_ADDR |
FUNCTIONFS_EVENTFD |
- FUNCTIONFS_ALL_CTRL_RECIP)) {
+ FUNCTIONFS_ALL_CTRL_RECIP |
+ FUNCTIONFS_CONFIG0_SETUP)) {
ret = -ENOSYS;
goto error;
}
@@ -3022,7 +3023,7 @@ static bool ffs_func_req_match(struct usb_function *f,
{
struct ffs_function *func = ffs_func_from_usb(f);
- if (config0)
+ if (config0 && !(func->ffs->user_flags & FUNCTIONFS_CONFIG0_SETUP))
return false;
switch (creq->bRequestType & USB_RECIP_MASK) {
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 93da4ca..acc6369 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -22,6 +22,7 @@ enum functionfs_flags {
FUNCTIONFS_VIRTUAL_ADDR = 16,
FUNCTIONFS_EVENTFD = 32,
FUNCTIONFS_ALL_CTRL_RECIP = 64,
+ FUNCTIONFS_CONFIG0_SETUP = 128,
};
/* Descriptor of an non-audio endpoint */
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usb: gadget: f_fs: handle control requests not directed to interface or endpoint
Introduces a new FunctionFS descriptor flag named
FUNCTIONFS_ALL_CTRL_RECIP. When this flag is enabled, control requests,
which are not explicitly directed to an interface or endpoint, can be
handled.
This allows FunctionFS userspace drivers to process non-standard
control requests.
Signed-off-by: Felix Hädicke
---
drivers/usb/gadget/function/f_fs.c | 34 ++
include/uapi/linux/usb/functionfs.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 73515d5..9eb4003 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -98,6 +98,8 @@ static int ffs_func_set_alt(struct usb_function *, unsigned,
unsigned);
static void ffs_func_disable(struct usb_function *);
static int ffs_func_setup(struct usb_function *,
const struct usb_ctrlrequest *);
+static bool ffs_func_req_match(struct usb_function *,
+ const struct usb_ctrlrequest *);
static void ffs_func_suspend(struct usb_function *);
static void ffs_func_resume(struct usb_function *);
@@ -2122,7 +2124,8 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
FUNCTIONFS_HAS_SS_DESC |
FUNCTIONFS_HAS_MS_OS_DESC |
FUNCTIONFS_VIRTUAL_ADDR |
- FUNCTIONFS_EVENTFD)) {
+ FUNCTIONFS_EVENTFD |
+ FUNCTIONFS_ALL_CTRL_RECIP)) {
ret = -ENOSYS;
goto error;
}
@@ -2974,8 +2977,9 @@ static int ffs_func_setup(struct usb_function *f,
* handle them. All other either handled by composite or
* passed to usb_configuration->setup() (if one is set). No
* matter, we will handle requests directed to endpoint here
-* as well (as it's straightforward) but what to do with any
-* other request?
+* as well (as it's straightforward). Other request recipient
+* types are only handled when the user flag FUNCTIONFS_ALL_CTRL_RECIP
+* is being used.
*/
if (ffs->state != FFS_ACTIVE)
return -ENODEV;
@@ -2996,7 +3000,10 @@ static int ffs_func_setup(struct usb_function *f,
break;
default:
- return -EOPNOTSUPP;
+ if (func->ffs->user_flags & FUNCTIONFS_ALL_CTRL_RECIP)
+ ret = le16_to_cpu(creq->wIndex);
+ else
+ return -EOPNOTSUPP;
}
spin_lock_irqsave(&ffs->ev.waitq.lock, flags);
@@ -3008,6 +3015,24 @@ static int ffs_func_setup(struct usb_function *f,
return 0;
}
+static bool ffs_func_req_match(struct usb_function *f,
+ const struct usb_ctrlrequest *creq)
+{
+ struct ffs_function *func = ffs_func_from_usb(f);
+
+ switch (creq->bRequestType & USB_RECIP_MASK) {
+ case USB_RECIP_INTERFACE:
+ return ffs_func_revmap_intf(func,
+ le16_to_cpu(creq->wIndex) >= 0);
+ case USB_RECIP_ENDPOINT:
+ return ffs_func_revmap_ep(func,
+ le16_to_cpu(creq->wIndex) >= 0);
+ default:
+ return (bool) (func->ffs->user_flags &
+ FUNCTIONFS_ALL_CTRL_RECIP);
+ }
+}
+
static void ffs_func_suspend(struct usb_function *f)
{
ENTER();
@@ -3258,6 +3283,7 @@ static struct usb_function *ffs_alloc(struct
usb_function_instance *fi)
func->function.set_alt = ffs_func_set_alt;
func->function.disable = ffs_func_disable;
func->function.setup = ffs_func_setup;
+ func->function.req_match = ffs_func_req_match;
func->function.suspend = ffs_func_suspend;
func->function.resume = ffs_func_resume;
func->function.free_func = ffs_free;
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 108dd79..93da4ca 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -21,6 +21,7 @@ enum functionfs_flags {
FUNCTIONFS_HAS_MS_OS_DESC = 8,
FUNCTIONFS_VIRTUAL_ADDR = 16,
FUNCTIONFS_EVENTFD = 32,
+ FUNCTIONFS_ALL_CTRL_RECIP = 64,
};
/* Descriptor of an non-audio endpoint */
--
2.8.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
