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]

Date:   Fri, 26 Jan 2018 11:42:36 +0100
From:   Tomas Paukrt <tomas.paukrt@...antech-bb.cz>
To:     Maxim Uvarov <muvarov@...il.com>, Bin Liu <b-liu@...com>,
        Yegor Yefremov <yegorslists@...glemail.com>,
        kernel list <linux-kernel@...r.kernel.org>,
        linux-usb <linux-usb@...r.kernel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        sergei.shtylyov@...entembedded.com
Subject: Re: [PATCHv2] musb_host: fix lockup on rxcsr_h_error

Hi Maxim,

unfortunately we cannot test the latest kernel right now, because we 
have custom drivers and additional changes that need to be ported, but 
the MUSB driver in our kernel should contain all fixes from 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/drivers/usb/musb

Best regards

Tomas


Dne 25.1.2018 v 17:24 Maxim Uvarov napsal(a):
> [1] says that issue is with back ported driver to 3.12.10. Can the
> latest kernel be tested on the same hw?
>
> Maxim.
>
> 2018-01-25 18:45 GMT+03:00 Bin Liu <b-liu@...com>:
>> 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.
>>
>
>