Hi Greg,
Thanks for the reply :)
On 2020/5/6 2:10, Greg KH wrote:
On Tue, May 05, 2020 at 10:21:10PM +0800, Jia-Ju Bai wrote:
In this case, "buffer[4] - 1 < ARRAY_SIZE(rc_keys)"
can be first satisfied, and then the value of buffer[4] can be changed
to a large number, causing a buffer-overflow vulnerability.
Um, maybe. I agree testing and then using the value can cause problems
when userspace provides you with that data and control, but for
DMA-backed memory, we are in so much other trouble if that is the case.
To avoid the risk of this vulnerability, buffer[4] is assigned to a
non-DMA local variable "index" at the beginning of
ttusb_dec_handle_irq(), and then this variable replaces each use of
buffer[4] in the function.
I strongly doubt this is even possible. Can you describe how you can
modify DMA memory and if so, would you do something tiny like this?
I have never modified DMA memory in the real world, but an attacker can
use a malicious device to do this.
There is a video that shows how to use the Inception tool to perform DMA
attacks and login in the Windows OS without password:
https://www.youtube.com/watch?v=HDhpy7RpUjM
Besides, the Windows OS resists against DMA attacks by disabling DMA of
external devices by default:
http://support.microsoft.com/kb/2516445
Considering that this patch is for a USB media driver, I think that an
attacker can just insert a malicious USB device to modify DMA memory and
trigger this bug.
Besides, not related to this patch, some drivers use DMA to send/receive
data (such as the URB used in USB drivers and ring descriptors used in
network drivers). In this case, if the data is malicious and used as an
array index through DMA, security problems may occur.
In my opinion, similar to the data from userspace, the data from
hardware may be also malicious and should be checked.
Maybe we could discuss this issue with DMA driver developers?
Best wishes,
Jia-Ju Bai
