linux-kernel - Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler

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:   Tue, 27 Dec 2016 11:52:15 +0100
From:   Janusz Dziedzic <janusz.dziedzic@...il.com>
To:     Baolin Wang <baolin.wang@...aro.org>
Cc:     balbi@...nel.org, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        linaro-kernel@...ts.linaro.org, broonie@...nel.org
Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt
 handler and irq thread handler

2016-12-26 9:01 GMT+01:00 Baolin Wang <baolin.wang@...aro.org>:
> On some platfroms(like x86 platform), when one core is running the USB gadget
> irq thread handler by dwc3_thread_interrupt(), meanwhile another core also can
> respond other interrupts from dwc3 controller and modify the event buffer by
> dwc3_interrupt() function, that will cause getting the wrong event count in
> irq thread handler to make the USB function abnormal.
>
> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this race.
>
Interesting, I always think we mask interrupt in dwc3_interrupt() by setting
DWC3_GEVNTSIZ_INTMASK
And unmask interrupt when we end dwc3_thread_interrupt().

So, we shouldn't get any IRQ from HW during dwc3_thread_interrupt(),
or I miss something?
Do you have some traces that indicate this masking will not work correctly?

BTW, what value you get when problem occured, 0xFFFC?

BR
Janusz

> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
> ---
>  drivers/usb/dwc3/gadget.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 6785595..1a1e1f4 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2894,10 +2894,13 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>                 return IRQ_HANDLED;
>         }
>
> +       spin_lock(&dwc->lock);
>         count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
>         count &= DWC3_GEVNTCOUNT_MASK;
> -       if (!count)
> +       if (!count) {
> +               spin_unlock(&dwc->lock);
>                 return IRQ_NONE;
> +       }
>
>         evt->count = count;
>         evt->flags |= DWC3_EVENT_PENDING;
> @@ -2914,6 +2917,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
>                 memcpy(evt->cache, evt->buf, count - amount);
>
>         dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
> +       spin_unlock(&dwc->lock);
>
>         return IRQ_WAKE_THREAD;
>  }
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Janusz Dziedzic