linux-kernel - Re: Regression: memory corruption on Atmel SAMA5D31

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 10 Mar 2022 11:40:23 +0100
From:   Peter Rosin <peda@...ntia.se>
To:     Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Tudor.Ambarus@...rochip.com,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Daniels Umanovskis <du@...ntia.se>,
        Patrice Vilchez <patrice.vilchez@...rochip.com>,
        Cristian Birsan <Cristian.Birsan@...rochip.com>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Saravana Kannan <saravanak@...gle.com>
Subject: Re: Regression: memory corruption on Atmel SAMA5D31

On 2022-03-10 10:58, Peter Rosin wrote:
> [bringing this threadlet back to the lists, hope that's ok]
> 
> On 2022-03-10 09:27, Nicolas Ferre wrote:
>>  From that article:
>> https://lwn.net/Articles/885941/
>>
>> I read:
>> 
>> "Koschel included a patch fixing a bug in the USB subsystem where the 
>> iterator passed to this macro was used after the exit from the macro, 
>> which is a dangerous thing to do. Depending on what happens within the 
>> list, the contents of that iterator could be something surprising, even 
>> in the absence of speculative execution. Koschel fixed the problem by 
>> reworking the code in question to stop using the iterator after the loop. "
>>
>> USB subsystem, "struct list_head *next, *prev;"... Some keywords present 
>> there... worth a try?
>>
>> Regards,
>>    Nicolas
> 
> gr_udc.c is not built with the config that is in use, which is sad because
> it looked like a good candidate.

at91_usba_udc.c, which is included, has the same pattern. But alas, doing
the equivalent patch there does not fix things either. I.e. (whitespace
damaged)

--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -863,6 +863,7 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
        struct usba_request *req;
        unsigned long flags;
        u32 status;
+       bool found = false;

        DBG(DBG_GADGET | DBG_QUEUE, "ep_dequeue: %s, req %p\n",
                        ep->ep.name, _req);
@@ -870,11 +871,13 @@ static int usba_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
        spin_lock_irqsave(&udc->lock, flags);

        list_for_each_entry(req, &ep->queue, queue) {
-               if (&req->req == _req)
+               if (&req->req == _req) {
+                       found = true;
                        break;
+               }
        }

-       if (&req->req != _req) {
+       if (!found) {
                spin_unlock_irqrestore(&udc->lock, flags);
                return -EINVAL;
        }

The test started out with 3 good hashes though, so I got my hopes up. But
no, it's about the same failure rate as usual. I have the feeling that I
will never again trust a single sha256sum...

Cheers,
Peter