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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Dec 15, 2020 at 11:54:08AM +0100, Felicitas Hetzelt wrote:
> 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?

ThunderClap was it!

https://lwn.net/Articles/786558/

Cc-ing Lu Baolu ..

Hm, this was a year ago and it looks like there are some extra SWIOTLB
patches to be done ?

> 
> 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&data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=THAJlYGLSOx3bKQYH62TLKH50By7Wnsu0z92snfNY84%3D&reserved=0
> >>>
> >>> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fen%2Fprocessors%2Famd-secure-encrypted-virtualization&data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=M3jmYCWaEvmAzIy%2F4z5XstsPf812SbEkuNX5PVVr0HY%3D&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&data=04%7C01%7Cashish.kalra%40amd.com%7C1d1cbca182a84c0e504708d8a079eec0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637435792867090126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=j0CIi%2F8hBkVx45XGBtT4Ri52uWIOdOts%2BSbJ0kCB5B0%3D&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!
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization