Re: [PATCH] media: usb: ttusb-dec: avoid buffer overflow in ttusb_dec_handle_irq() when DMA failures/attacks occur

[Greg KH <gregkh@linuxfoundation.org>]

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