linux-kernel - Re: [PATCH] usb: storage: alauda: fix possible buffer overflow casued by bad DMA value in alauda_read

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>] [day] [month] [year] [list]

Message-ID: <20200530155927.GC29298@rowland.harvard.edu>
Date:   Sat, 30 May 2020 11:59:27 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Jia-Ju Bai <baijiaju@...nghua.edu.cn>
Cc:     gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        usb-storage@...ts.one-eyed-alien.net, linux-kernel@...r.kernel.org,
        Jia-Ju Bai <baijiaju1990@...il.com>
Subject: Re: [PATCH] usb: storage: alauda: fix possible buffer overflow
 casued by bad DMA value in alauda_read_map()

On Sat, May 30, 2020 at 10:42:30PM +0800, Jia-Ju Bai wrote:
> From: Jia-Ju Bai <baijiaju1990@...il.com>
> 
> The value us->iobuf is stored in DMA memory, and it is assigned to data,
> so data[6] and data[7] can be modified at anytime by malicious hardware.
> In this case, data[6] ^ data[7] can be a quite large number, which may 
> cause buffer overflow when the code "parity[data[6] ^ data[7]]" is
> executed.
> 
> To fix this possible bug, data[6] ^ data[7] is assigned to a local
> variable, and then this variable is checked before being used.

There are much worse problems than this in the alauda driver.  For 
example, alauda_get_redu_data() does I/O from a data buffer on the 
stack; this is not allowed.  That's just the example I noticed; there 
may very well be others.

If you want to fix something, fix that.

If you're still worried about malicious hardware, the way to fix the 
problem is not to change this one location.  Instead, you should modify 
the driver so that us->iobuf is not stored in DMA memory.

Alan Stern