linux-kernel - Re: [PATCHv2] musb_host: fix lockup on rxcsr_h

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: <20180125154508.GB6590@uda0271908>
Date:   Thu, 25 Jan 2018 09:45:08 -0600
From:   Bin Liu <b-liu@...com>
To:     Yegor Yefremov <yegorslists@...glemail.com>
CC:     Maxim Uvarov <muvarov@...il.com>,
        kernel list <linux-kernel@...r.kernel.org>,
        linux-usb <linux-usb@...r.kernel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        <sergei.shtylyov@...entembedded.com>,
        Tomas Paukrt <tomas.paukrt@...antech-bb.cz>
Subject: Re: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

Hi Yegor and Max,

On Tue, May 03, 2016 at 04:25:58PM +0200, Yegor Yefremov wrote:
> On Tue, May 3, 2016 at 3:48 PM, Bin Liu <b-liu@...com> wrote:
> > Hi,
> >
> > On Tue, May 03, 2016 at 12:03:52PM +0200, Yegor Yefremov wrote:
> >> On Thu, Apr 28, 2016 at 4:37 PM, Bin Liu <b-liu@...com> wrote:
> >> > Hi,
> >> >
> >> > On Thu, Apr 28, 2016 at 09:51:37AM +0300, Maxim Uvarov wrote:
> >> >
> >> > [snip]
> >> >
> >> >> Hello Bin,
> >> >>
> >> >> yes, it also works with that reset and go to finish:
> >> >>
> >> >> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> >> >> index c3d5fc9..8cd98e7 100644
> >> >> --- a/drivers/usb/musb/musb_host.c
> >> >> +++ b/drivers/usb/musb/musb_host.c
> >> >> @@ -1599,6 +1599,10 @@ void musb_host_rx(struct musb *musb, u8 epnum)
> >> >>                 status = -EPROTO;
> >> >>                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
> >> >>
> >> >> +               rx_csr &= ~MUSB_RXCSR_H_ERROR;
> >> >> +               musb_writew(epio, MUSB_RXCSR, rx_csr);
> >> >> +
> >> >> +               goto finish;
> >> >>         } else if (rx_csr & MUSB_RXCSR_DATAERROR) {
> >> >>
> >> >>                 if (USB_ENDPOINT_XFER_ISOC != qh->type) {
> >> >>
> >> >
> >> > Thanks for testing it.
> >>
> >> Have tested your patch and now both FT4232 and Huawei don't freeze on removal.
> >>
> >> Bin, Max thanks for fixing this issue.
> >>
> >> Tested-by: Yegor Yefremov <yegorslists@...glemail.com>
> >
> > Thanks for testing.
> >
> > Can you please test the patch [1] instead? I'd like to use it as the
> > fix.
> >
> > [1] http://marc.info/?l=linux-usb&m=146222355213935&w=2
> 
> The patch behaves the same as the previous one.

Sorry for bringing up this old thread, but it seems to be too aggressive
to stop scheduling further urbs on errors [1]. So is it possible for you
to re-test your usecase by reverting commit

    dbac5d07d13e ("usb: musb: host: don't start next rx urb if current one failed")

to see if only commit

    b5801212229f ("usb: musb: host: clear rxcsr error bit if set")

itself solves your issue?

I know you have tested the patch in [2], which is similar to commit
b5801212229f, but tha latter doesn't have 'goto finish' which does dma
cleanup on errors, it makes more sense to me. But I'd like to have you
tested with reverting dbac5d07d13e to be sure.

[1] https://marc.info/?l=linux-usb&m=151689238420622&w=2
[2] https://marc.info/?l=linux-kernel&m=146185425805967&w=2

thanks,
-Bin.