| 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
| ||
|
Date: Fri, 9 Sep 2016 12:12:04 -0400 (EDT)
From: Alan Stern <stern@...land.harvard.edu>
To: Felipe Balbi <felipe.balbi@...ux.intel.com>
cc: Peter Zijlstra <peterz@...radead.org>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Ingo Molnar <mingo@...hat.com>,
USB list <linux-usb@...r.kernel.org>,
Kernel development list <linux-kernel@...r.kernel.org>,
Will Deacon <will.deacon@....com>
Subject: Re: Memory barrier needed with wake_up_process()?
On Fri, 9 Sep 2016, Felipe Balbi wrote:
> Finally :-) Here's the diff I used:
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 8f3659b65f53..0716024f6b65 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -481,6 +481,8 @@ static void bulk_out_complete(struct usb_ep *ep, struct usb_request *req)
> spin_lock(&common->lock);
> bh->outreq_busy = 0;
> bh->state = BUF_STATE_FULL;
> + if (bh->bulk_out_intended_length == US_BULK_CB_WRAP_LEN)
> + trace_printk("compl: bh %p state %d\n", bh, bh->state);
> wakeup_thread(common);
> spin_unlock(&common->lock);
> }
> @@ -2208,6 +2210,7 @@ static int get_next_command(struct fsg_common *common)
> rc = sleep_thread(common, true);
> if (rc)
> return rc;
> + trace_printk("next: bh %p state %d\n", bh, bh->state);
> }
> smp_rmb();
> rc = fsg_is_set(common) ? received_cbw(common->fsg, bh) : -EIO;
>
>
> And here's trace output:
>
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 1002/1002 #P:4
> #
> # _-----=> irqs-off
> # / _----=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / delay
> # TASK-PID CPU# |||| TIMESTAMP FUNCTION
> # | | | |||| | |
> file-storage-3578 [000] .... 21166.789127: fsg_main_thread: next: bh ffff880111e69a00 state 2
> file-storage-3578 [000] .... 21166.789312: fsg_main_thread: next: bh ffff880111e69a00 state 2
> irq/17-dwc3-3579 [003] d..1 21166.789395: bulk_out_complete: compl: bh ffff880111e69a00 state 1
> file-storage-3578 [000] .... 21166.789445: fsg_main_thread: next: bh ffff880111e69a00 state 1
Okay, that's normal. 2 = BUF_STATE_BUSY, 1 = BUF_STATE_FULL. So we get woken
up a couple of times while the transfer is in progress (probably because some
earlier buffers have finished transferring), then the CBW transfer completes
and the buffer is read.
...
> file-storage-3578 [002] .... 21167.726827: fsg_main_thread: next: bh ffff880111e69a80 state 2
> irq/17-dwc3-3579 [003] d..1 21167.727066: bulk_out_complete: compl: bh ffff880111e69a80 state 1
> file-storage-3578 [002] .... 21167.727072: fsg_main_thread: next: bh ffff880111e69a80 state 1
> file-storage-3578 [002] .... 21167.729458: fsg_main_thread: next: bh ffff880111e6aac0 state 2
> irq/17-dwc3-3579 [003] d..1 21167.729666: bulk_out_complete: compl: bh ffff880111e6aac0 state 1
> file-storage-3578 [002] .... 21167.729670: fsg_main_thread: next: bh ffff880111e6aac0 state 1
And this is where everything stopped?
This also looks normal. So the question is what happened when
get_next_command() returned after this?
Felipe, maybe the patch below (in place of your current patch) will
help. Since the events that it logs are all supposed to be unusual,
you can use printk if you want, but I wrote it with trace_printk.
Alan Stern
Index: usb-4.x/drivers/usb/gadget/function/f_mass_storage.c
===================================================================
--- usb-4.x.orig/drivers/usb/gadget/function/f_mass_storage.c
+++ usb-4.x/drivers/usb/gadget/function/f_mass_storage.c
@@ -415,6 +415,7 @@ static void raise_exception(struct fsg_c
* If a lower-or-equal priority exception is in progress, preempt it
* and notify the main thread by sending it a signal.
*/
+ trace_printk("raise_exception %d\n", new_state);
spin_lock_irqsave(&common->lock, flags);
if (common->state <= new_state) {
common->exception_req_tag = common->ep0_req_tag;
@@ -2495,6 +2496,7 @@ static void handle_exception(struct fsg_
static int fsg_main_thread(void *common_)
{
struct fsg_common *common = common_;
+ int rc;
/*
* Allow the thread to be killed by a signal, but set the signal mask
@@ -2518,6 +2520,7 @@ static int fsg_main_thread(void *common_
/* The main loop */
while (common->state != FSG_STATE_TERMINATED) {
if (exception_in_progress(common) || signal_pending(current)) {
+ trace_printk("handling exception\n");
handle_exception(common);
continue;
}
@@ -2527,24 +2530,38 @@ static int fsg_main_thread(void *common_
continue;
}
- if (get_next_command(common))
+ rc = get_next_command(common);
+ if (rc) {
+ trace_printk("get_next_command -> %d\n", rc);
continue;
+ }
spin_lock_irq(&common->lock);
if (!exception_in_progress(common))
common->state = FSG_STATE_DATA_PHASE;
spin_unlock_irq(&common->lock);
- if (do_scsi_command(common) || finish_reply(common))
+ rc = do_scsi_command(common);
+ if (rc) {
+ trace_printk("do_scsi_command -> %d\n", rc);
+ continue;
+ }
+ rc = finish_reply(common);
+ if (rc) {
+ trace_printk("finish_reply -> %d\n", rc);
continue;
+ }
spin_lock_irq(&common->lock);
if (!exception_in_progress(common))
common->state = FSG_STATE_STATUS_PHASE;
spin_unlock_irq(&common->lock);
- if (send_status(common))
+ rc = send_status(common);
+ if (rc) {
+ trace_printk("send_status -> %d\n", rc);
continue;
+ }
spin_lock_irq(&common->lock);
if (!exception_in_progress(common))
Powered by blists - more mailing lists