linux-kernel - Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass

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: <Pine.LNX.4.44L0.1406031033010.1054-100000@iolanthe.rowland.org>
Date:	Tue, 3 Jun 2014 10:48:45 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Wei.Yang@...driver.com, Michal Nazarewicz <mina86@...a86.com>,
	Andrzej Pietrasiewicz <andrzej.p@...sung.com>
cc:	Felipe Balbi <balbi@...com>, <gregkh@...uxfoundation.org>,
	USB list <linux-usb@...r.kernel.org>,
	Kernel development list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage

On Tue, 3 Jun 2014 Wei.Yang@...driver.com wrote:

> From: Yang Wei <Wei.Yang@...driver.com>
> 
> While loading g_mass_storage module, the following warning is triggered.
> In fact, it is more easy to reproduce it with RT kernel.
> 
> WARNING: at drivers/usb/gadget/composite.c:
> usb_composite_setup_continue: Unexpected call
> Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage
> [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24)
> [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74)
> [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48)
> [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage])
> [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage])
> [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage])
> [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c)
> [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) 
> 
> The root cause just likes the following scenario.
> 
> irq thread
> 
> composite_disconnect()
> |
> |->fsg_disable() fsg->common->new_fsg = NULL
>                  and then wake fsg_main_thread
>                  with seting common->state to
>                  FSG_STATE_CONFIG_CHANGE.
>                                                     fsg_main_thread
>                                                     |
>                                                     |->do_set_interface()
> irq thread
> 
> set_config()
> |
> |->fsg_set_alt() fsg->common->new_fsg = new_fsg
>                  and then also wake up fsg_main_thread
>                  with setting common->state to
>                  FSG_STATE_CONFIG_CHANGE.
>                                                     |-> if(common->new_fsg)
> 		                                                 usb_composite_setup_continue()
> 
> In this case, fsg_main_thread would invoke usb_composite_setup_continue
> twice, so the second call would trigger the above call trace, as we also
> save common->new_fsg while changing the common->state.

Michal and Andrzej:

I haven't paid much attention to these matters, because you handled the 
conversion from g_file_storage to f_mass_storage using the composite 
framework.  But this patch seemed odd, so I took a closer look.

In f_mass_storage.c, struct fsg_common is shared among all the function
instances.  This structure includes things like cmnd and nluns, which
will in general be different for different functions.

That's okay if each function is in a separate config, but what happens
when there are multiple functions in the same config, using different
interfaces?  What if the host sends concurrent commands to two of these
functions?

Am I missing something?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/