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

From: Jason Wang <jasowang@redhat.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: ashish kalra <ashish.kalra@amd.com>,
	Felicitas Hetzelt <file@sect.tu-berlin.de>,
	Martin Radev <martin.radev@aisec.fraunhofer.de>,
	david kaplan <david.kaplan@amd.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	virtualization@lists.linux-foundation.org,
	Robert Buhren <robert@sect.tu-berlin.de>,
	iommu@lists.linux-foundation.org,
	Mathias Morbitzer <mathias.morbitzer@aisec.fraunhofer.de>,
	hch@lst.de
Subject: Re: swiotlb/virtio: unchecked device dma address and length
Date: Wed, 16 Dec 2020 01:41:48 -0500 (EST)	[thread overview]
Message-ID: <613214145.36646222.1608100908658.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <1330503106.36174346.1608098013639.JavaMail.zimbra@redhat.com>

----- Original Message -----
> 
> 
> ----- Original Message -----
> > .snip.
> > > > > 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.
> > > 
> > > 
> > > I'm not sure this can fix all the cases. It looks to me we should map
> > > descriptor coherent but readonly (which is not supported by current DMA
> > > API).
> > 
> > I think I am missing something obvious here. The attacker is the
> > hypervisor,
> > aka
> > the owner of the VirtIO device (ring0). The attacker is the one that
> > provides the addr/len - having that readonly from a guest perspective
> > does not change the fact that the hypervisor can modify the memory range
> > by mapping it via a different virtual address in the hypervisor? (aka
> > aliasing it).
> 
> Right, but if we allow hypervisor to provide arbitrary addr/len, does
> it mean hypervisor can read encrypted content of encrypted memory of
> guest through swiotlb?
> 
> Thanks

Actually not. I think you're right.

Thanks

> 
> > > 
> > > Otherwise, device can modify the desc[i].addr/desc[i].len at any time to
> > > pretend a valid mapping.
> > 
> > With the swiotlb=force as long as addr/len are within the PA boundaries
> > within the SWIOTLB pool this should be OK?
> > 
> > After all that whole area is in cleartext and visible to the attacker.
> > 
> > 
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization