Message-ID: <CAOZ5it0Yj8taexnFJiAqJhPP_8z5Dv_OjonyGXMbA3Tiu=9ugQ@mail.gmail.com>
Date: Fri, 22 Nov 2024 13:33:15 -0700
From: Brian Johannesmeyer <bjohannesmeyer@...il.com>
To: Tianyu Lan <Tianyu.Lan@...rosoft.com>, Christoph Hellwig <hch@....de>, 
	Marek Szyprowski <m.szyprowski@...sung.com>, Robin Murphy <robin.murphy@....com>, iommu@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Cc: Raphael Isemann <teemperor@...il.com>, Cristiano Giuffrida <giuffrida@...vu.nl>, Herbert Bos <h.j.bos@...nl>, 
	Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [RFC 0/1] swiotlb: Mitigate potential DoS caused by BUG_ON()

On Fri, Nov 22, 2024 at 12:13 PM Brian Johannesmeyer
<bjohannesmeyer@...il.com> wrote:
>
> We identified a security issue in the swiotlb unmapping operation, stemming
> from the way some drivers save streaming DMA addresses. This issue can
> potentially be exploited by a malicious peripheral device to cause a
> denial-of-service (DoS) via a kernel panic.
>
> **Disclosure Context**
> We previously reported a similar issue to the Linux kernel security team.
> However, they advised submitting such cases directly to the relevant
> subsystem maintainers and the hardening mailing list, because Linux
> implicitly assumes hardware can be trusted.
>
> **Threat Model**
> While Linux drivers typically trust their hardware, there may be specific
> drivers that do not operate under this assumption. Hence, we consider a
> malicious peripheral device that corrupts DMA data to exploit the kernel.
> In this scenario, the device manipulates driver-initialized data (similar
> to the attack described in the Thunderclap paper [0]) to achieve a DoS.
>
> **Background**
> Streaming DMA is often used as follows:
> (1) A driver maps a buffer to DMA using dma_map_single(), which returns a
> DMA address. This address is then saved by the driver for later use.
> (2) When the buffer is no longer needed, the driver unmaps it using
> dma_unmap_single(), passing the previously saved DMA address.
> (3) Under certain conditions---such as the driver using direct mapping
> operations, and the mapped buffer requiring a swiotlb
> buffer---dma_unmap_single() calls swiotlb_release_slots(). Here, the saved
> DMA address is passed as its tlb_addr argument.
>
> **Vulnerability**
> It is common for drivers to store the DMA address returned by
> dma_map_single() into a coherent DMA region, which can be manipulated by a
> malicious device. For example, the E100 driver and RealTek 8139C+ driver
> map socket buffers into streaming DMA and save their DMA addresses to
> coherent DMA data. While these drivers might assume trusted hardware, this
> behavior is not necessarily unique to them.
>
> If an untrusted device corrupts the DMA address, it can influence the
> tlb_addr argument passed to swiotlb_release_slots(). Inside this function,
> tlb_addr is used to calculate aindex, which is validated via BUG_ON(aindex
> >= mem->nareas). By manipulating the DMA address, an attacker can trigger
> this assertion, resulting in a kernel panic and DoS.
>
> I have a PDF document that illustrates how these steps work. Please let me
> know if you would like me to share it with you.
>
> **Proposed mitigation**
> To address this issue, two potential approaches are possible:
>
> (1) Mitigating the *initialization* of attacker data: Prevent drivers from
> saving critical data, such as DMA addresses, in attacker-controllable
> regions.
> (2) Mitigating the *use* of attacker data: Modify the handling of critical
> data, such as in the BUG_ON() statement, to not result in catastrophic
> outcomes like kernel panics.
>
> While approach (1) is more complete, it is more challenging to deploy
> universally. Hence, we propose mitigating this issue with approach (2):
> i.e., replacing the BUG_ON() assertion with more graceful error handling.
> The attached patch implements this change, ensuring the kernel can handle
> the condition safely without triggering a panic.
>
> **Request for Feedback**
> I am not deeply familiar with the swiotlb internals, so I would appreciate
> any feedback on the patch. In particular, is there a more graceful way to
> handle the error than returning?
>
> Thanks,
>
> Brian Johannesmeyer
>
> [0] Link: https://www.csl.sri.com/~neumann/ndss-iommu.pdf
>
> Brian Johannesmeyer (1):
>   swiotlb: Replace BUG_ON() with graceful error handling
>
>  kernel/dma/swiotlb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> --
> 2.34.1
>

Whoops -- didn't send to the hardening mailing list. Adding it now.

-Brian

Powered by blists - more mailing lists