On Wed, May 06, 2020 at 06:13:01PM +0800, Jia-Ju Bai wrote: > 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 If you have control over the hardware, and can write to any DMA memory, again, there's almost nothing a kernel can do to protect from that. > Besides, the Windows OS resists against DMA attacks by disabling DMA of > external devices by default: > http://support.microsoft.com/kb/2516445 So does Linux :) > 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. USB devices do not touch DMA memory so they physically can not do things like what thunderbolt devices can do. > 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? Sure, but I think that's outside the scope of this tiny patch :) thanks, greg k-h
