Re: swiotlb/virtio: unchecked device dma address and length

From: Felicitas Hetzelt <file@sect.tu-berlin.de>
To: Ashish Kalra <ashish.kalra@amd.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Thomas.Lendacky@amd.com, brijesh.singh@amd.com, "Radev,
	Martin" <martin.radev@aisec.fraunhofer.de>,
	david.kaplan@amd.com, "Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Jon.Grimm@amd.com, virtualization@lists.linux-foundation.org,
	Robert Buhren <robert@sect.tu-berlin.de>,
	iommu@lists.linux-foundation.org, "Morbitzer,
	Mathias" <mathias.morbitzer@aisec.fraunhofer.de>,
	hch@lst.de
Subject: Re: swiotlb/virtio: unchecked device dma address and length
Date: Tue, 15 Dec 2020 11:54:08 +0100	[thread overview]
Message-ID: <c90f5ea4-b8b2-98d7-546a-dc71fb618230@sect.tu-berlin.de> (raw)
In-Reply-To: <20201215084720.GA9981@ashkalra_ubuntu_server>

Hello,
thank you all for looking into this! To answer some of the questions:
 - Did you have already some PoC fixes for this:
   We don't have a full PoC or fix currently. Thought we have a PoC
   with which were able to overwrite memory outside of the mapped
   dma region.
 - Is there a CVE associated with this?
   No
 - Is there a paper on this you all are working on?
   Yes, we were planning to use this bug (among others
   in a paper)

Could you point us to the intel thunder issue that you mentioned?

On 12/15/20 9:47 AM, Ashish Kalra wrote:
> On Mon, Dec 14, 2020 at 04:49:50PM -0500, Konrad Rzeszutek Wilk wrote:
>> On Fri, Dec 11, 2020 at 06:31:21PM +0100, Felicitas Hetzelt wrote:
>>> Hello,
>>
>> Hi! Please see below my responses.
>>
>>> we have been analyzing the Hypervisor-OS interface of Linux
>>> and discovered bugs in the swiotlb/virtio implementation that can be
>>> triggered from a malicious Hypervisor / virtual device.
>>> With SEV, the SWIOTLB implementation is forcefully enabled and would
>>> always be used. Thus, all virtio devices and others would use it under
>>> the hood.
>>>
>>> The reason for analyzing this interface is that, technologies such as
>>> Intel's Trusted Domain Extensions [1] and AMD's Secure Nested Paging [2]
>>> change the threat model assumed by various Linux kernel subsystems.
>>> These technologies take the presence of a fully malicious hypervisor
>>> into account and aim to provide protection for virtual machines in such
>>> an environment. Therefore, all input received from the hypervisor or an
>>> external device should be carefully validated. Note that these issues
>>> are of little (or no) relevance in a "normal" virtualization setup,
>>> nevertheless we believe that it is required to fix them if TDX or SNP is
>>> used.
>>>
>>> We are happy to provide more information if needed!
>>>
>>> [1]
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftware.intel.com%2Fcontent%2Fwww%2Fus%2Fen%2Fdevelop%2Farticles%2Fintel-trust-domain-extensions.html&amp;data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=THAJlYGLSOx3bKQYH62TLKH50By7Wnsu0z92snfNY84%3D&amp;reserved=0
>>>
>>> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fen%2Fprocessors%2Famd-secure-encrypted-virtualization&amp;data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=M3jmYCWaEvmAzIy%2F4z5XstsPf812SbEkuNX5PVVr0HY%3D&amp;reserved=0
>>>
>>> Bug:
>>> OOB memory write.
>>> dma_unmap_single -> swiotlb_tbl_unmap_single is invoked with dma_addr
>>> and length parameters that are under control of the device.
>>> This happens e.g. in virtio_ring:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.10-rc7%2Fsource%2Fdrivers%2Fvirtio%2Fvirtio_ring.c%23L378&amp;data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=j0CIi%2F8hBkVx45XGBtT4Ri52uWIOdOts%2BSbJ0kCB5B0%3D&amp;reserved=0
>>
>> Heya!
>>
>> Thank you for pointing this out! I've a couple of questions and hope you can
>> help me out with them.
>>
>> Also CC-ing AMD / TDX folks.
>>>
> 
> Adding more relevant folks in AMD.
> 
> Needless to say, the swiotlb code needs to validate this external untrusted input dma_addr and length parameters.
> 
> Thanks,
> Ashish
> 
>>> This raises two issues:
>>> 1) swiotlb_tlb_unmap_single fails to check whether the index generated
>>> from the dma_addr is in range of the io_tlb_orig_addr array.
>>
>> That is fairly simple to implement I would think. That is it can check
>> that the dma_addr is from the PA in the io_tlb pool when SWIOTLB=force
>> is used.
>>
>>> 2) when swiotlb_bounce is called the device controls the length of the
>>> memory copied to the cpu address.
>>
>> So.. this sounds very similar to the Intel Thunder.. something issue
>> where this exact issue was fixed by handing the DMA off to the SWIOTLB
>> bounce code.
>>
>> But if that is broken, then that CVE is still not fixed?
>>
>> So the issue here is that swiotlb_tbl_unmap_single(..,mapping_size,) is
>> under the attacker control. Ugh.
>>
>> One way could be to have a io_tlb_orig_addr-ish array with the length
>> of mappings to double check?
>>
>> Couple more questions:
>>  - Did you have already some PoC fixes for this? 
>>  - Is there a CVE associated with this?
>>  - Is there a paper on this you all are working on?
>>
>> Thank you!
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu