----- 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? > > ThanksActually 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. > > > > >
Konrad Rzeszutek Wilk
2020-Dec-16 13:04 UTC
swiotlb/virtio: unchecked device dma address and length
On December 16, 2020 1:41:48 AM EST, Jason Wang <jasowang at redhat.com> wrote:> > >----- 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?Yes .>> >> Thanks > >Actually not. I think you're right.Your sentence is very confusing. On a DMA unmap SWIOTLB (when force is used) it trusts the driver from providing the correct DMA address and length which SWIOTLB uses to match to its associated original PA address. Think original PA having a mapping to a PA in the SWIOTLB pool. The length is not checked so the attacker can modify that to say a huge number and cause SWIOTLB bounce code to write or read data from non SWIOTLB PA into the SWIOTLB PA pool.> >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. >> > >> > >>