Hello everybody,
I will try help clarify some things.
> 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.
> 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.
This is true.
As an example, I attached to the QEMU process, set a BP to
`virtqueue_split_fill`
and modified the length field from 0x40 to 0x10000, and filled the corresponding
buffer in the swiotlb region with As (0x41).
Immediately after resuming execution, the kernel would crash:
[ 122.154142] general protection fault, probably for non-canonical address
0x4141414141414141: 0000 [#1] PREEMPT SMP NOPTI
[ 122.156088] CPU: 0 PID: 917 Comm: kworker/0:6 Kdump: loaded Tainted: G
W E 5.6.12-sevault+ #28
[ 122.157855] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0
02/06/2015
[ 122.159079] Workqueue: events_freezable_power_ disk_events_workfn
[ 122.160040] RIP: 0010:scsi_queue_rq+0x5af/0xa70 [scsi_mod]
[ 122.160916] Code: 01 89 83 9c 02 00 00 41 80 7f 08 00 74 07 83 8b 9c 02 00
00 08 48 8b 83 40 02 00 00 c7 83 3c 01 00 00 00 00 00 00 48 8d 78 08 <48>
c7 00 00 00 00 00 48 c7 40 58 00 00 00 00 48 83 e7 f8 48
29 f8
[ 122.163821] RSP: 0018:ffffc900002efb08 EFLAGS: 00010202
[ 122.164637] RAX: 4141414141414141 RBX: ffff888035b89c00 RCX: ffff888035b89ed0
[ 122.165775] RDX: 0000000000000008 RSI: 0000000000000000 RDI: 4141414141414149
[ 122.166891] RBP: ffff888035946000 R08: ffff888035a79860 R09: 0000000000000000
[ 122.168016] R10: ffffea0001287280 R11: 0000000000000008 R12: ffff888035b89d18
[ 122.169159] R13: ffff888035945000 R14: ffff888035946000 R15: ffffc900002efba0
[ 122.170287] FS: 0000000000000000(0000) GS:ffff88807f800000(0000)
knlGS:0000000000000000
[ 122.171564] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 122.172470] CR2: 0000560e654b77b8 CR3: 000000004dd38000 CR4: 00000000003406f0
What and where gets overwritten entirely depends on what virtio driver is being
targeted. All manage their memory for the descriptor buffers differently so the
overwrite
may require to be large.
In the context of VirtIO and SWIOTLB, there are also these three fields other
than the length:
dma_addr, flags, next
I had a look around a little bit, so my take is the following:
1) There's already validation for dma_addr before doing the unmap with a call
to is_swiotlb_buffer (1). I think this check is sufficient.
2) flags
Before doing the unmap, the virtio implementation would check the flag and
based on it
would select a DMA direction (TO/FROM DEVICE). Still, it seems that this
would not
trick the driver to copy data to the device since only a `sync for CPU` may
be performed
in the unmap path.
However, I can see that this would eliminate a memcpy to any passed buffer
which means
that an uninitialized buffer may be passed to some client like a user space
application.
I tested that with the device `virtio-rng` which would leak some stored
kernel address.
3) next
I think this was reported by Felicitas that this can lead to an OOB access,
but it is limited.
So, I think that the problem of overwriting the addr and flags fields are not a
big issue
on their own. But there should be some validation for the size and that should
be probably
done the SWIOTLB implementation level.
There's already an array to keep the original address (3), What about adding an
extra array
to keep track of also the `original size`. It would be populated when some
memory is mapped,
just like with io_tlb_orig_addr (4). Then the validation can be added in (5)
and (6).
This swiotlb/virtio issue affects the AMD SEV features where swiotlb is always
forcefully enabled. I can also see that SWIOTLB is also always enabled for s390
but I don't
know their threat model.
[1] https://elixir.bootlin.com/linux/v5.10/source/include/linux/swiotlb.h#L72
[2]
https://elixir.bootlin.com/linux/v5.8/source/drivers/virtio/virtio_ring.c#L381
[3] https://elixir.bootlin.com/linux/v5.8/source/kernel/dma/swiotlb.c#L103
[4] https://elixir.bootlin.com/linux/v5.8/source/kernel/dma/swiotlb.c#L570
[5] https://elixir.bootlin.com/linux/v5.8/source/kernel/dma/swiotlb.c#L588
[6] https://elixir.bootlin.com/linux/v5.8/source/kernel/dma/swiotlb.c#L633
________________________________
From: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Sent: Wednesday, December 16, 2020 2:07 PM
To: Michael S. Tsirkin <m...@redhat.com>; Jason Wang <jasow...@redhat.com>
Cc: Felicitas Hetzelt <f...@sect.tu-berlin.de>; ashish.ka...@amd.com
<ashish.ka...@amd.com>; jun.nakaj...@intel.com <jun.nakaj...@intel.com>;
h...@lst.de <h...@lst.de>; virtualizat...@lists.linux-foundation.org
<virtualizat...@lists.linux-foundation.org>; iommu@lists.linux-foundation.org
<iommu@lists.linux-foundation.org>; Radev, Martin
<martin.ra...@aisec.fraunhofer.de>; Morbitzer, Mathias
<mathias.morbit...@aisec.fraunhofer.de>; Robert Buhren
<rob...@sect.tu-berlin.de>; david.kap...@amd.com <david.kap...@amd.com>
Subject: Re: 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).
>
>Neither is this supported but encrypted memory technologies.
-ECONFUSED.
Could you state this once more please? I am not exactly sure what you are saying
>
>> Otherwise, device can modify the desc[i].addr/desc[i].len at any time
>to
>> pretend a valid mapping.
>>
>> Thanks
>>
>>
>> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
