Virtualization - Dec 2020 - swiotlb/virtio: unchecked device dma address and length

.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).
> 
> 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.

----- 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 uncrypted content of encrypted memory of
guest through swiotlb?

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.
> 
>