Skip to content

Commit 04867a7

Browse files
willdeaconChristoph Hellwig
authored andcommitted
swiotlb: Fix double-allocation of slots due to broken alignment handling
Commit bbb73a1 ("swiotlb: fix a braino in the alignment check fix"), which was a fix for commit 0eee5ae ("swiotlb: fix slot alignment checks"), causes a functional regression with vsock in a virtual machine using bouncing via a restricted DMA SWIOTLB pool. When virtio allocates the virtqueues for the vsock device using dma_alloc_coherent(), the SWIOTLB search can return page-unaligned allocations if 'area->index' was left unaligned by a previous allocation from the buffer: # Final address in brackets is the SWIOTLB address returned to the caller | virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1645-1649/7168 (0x98326800) | virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1649-1653/7168 (0x98328800) | virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1653-1657/7168 (0x9832a800) This ends badly (typically buffer corruption and/or a hang) because swiotlb_alloc() is expecting a page-aligned allocation and so blindly returns a pointer to the 'struct page' corresponding to the allocation, therefore double-allocating the first half (2KiB slot) of the 4KiB page. Fix the problem by treating the allocation alignment separately to any additional alignment requirements from the device, using the maximum of the two as the stride to search the buffer slots and taking care to ensure a minimum of page-alignment for buffers larger than a page. This also resolves swiotlb allocation failures occuring due to the inclusion of ~PAGE_MASK in 'iotlb_align_mask' for large allocations and resulting in alignment requirements exceeding swiotlb_max_mapping_size(). Fixes: bbb73a1 ("swiotlb: fix a braino in the alignment check fix") Fixes: 0eee5ae ("swiotlb: fix slot alignment checks") Signed-off-by: Will Deacon <will@kernel.org> Reviewed-by: Michael Kelley <mhklinux@outlook.com> Reviewed-by: Petr Tesarik <petr.tesarik1@huawei-partners.com> Tested-by: Nicolin Chen <nicolinc@nvidia.com> Tested-by: Michael Kelley <mhklinux@outlook.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
1 parent b9fa169 commit 04867a7

File tree

1 file changed

+14
-12
lines changed

1 file changed

+14
-12
lines changed

kernel/dma/swiotlb.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
10041004
phys_to_dma_unencrypted(dev, pool->start) & boundary_mask;
10051005
unsigned long max_slots = get_max_slots(boundary_mask);
10061006
unsigned int iotlb_align_mask =
1007-
dma_get_min_align_mask(dev) | alloc_align_mask;
1007+
dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
10081008
unsigned int nslots = nr_slots(alloc_size), stride;
10091009
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
10101010
unsigned int index, slots_checked, count = 0, i;
@@ -1016,18 +1016,17 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
10161016
BUG_ON(area_index >= pool->nareas);
10171017

10181018
/*
1019-
* For allocations of PAGE_SIZE or larger only look for page aligned
1020-
* allocations.
1019+
* For mappings with an alignment requirement don't bother looping to
1020+
* unaligned slots once we found an aligned one.
10211021
*/
1022-
if (alloc_size >= PAGE_SIZE)
1023-
iotlb_align_mask |= ~PAGE_MASK;
1024-
iotlb_align_mask &= ~(IO_TLB_SIZE - 1);
1022+
stride = get_max_slots(max(alloc_align_mask, iotlb_align_mask));
10251023

10261024
/*
1027-
* For mappings with an alignment requirement don't bother looping to
1028-
* unaligned slots once we found an aligned one.
1025+
* For allocations of PAGE_SIZE or larger only look for page aligned
1026+
* allocations.
10291027
*/
1030-
stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
1028+
if (alloc_size >= PAGE_SIZE)
1029+
stride = umax(stride, PAGE_SHIFT - IO_TLB_SHIFT + 1);
10311030

10321031
spin_lock_irqsave(&area->lock, flags);
10331032
if (unlikely(nslots > pool->area_nslabs - area->used))
@@ -1037,11 +1036,14 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
10371036
index = area->index;
10381037

10391038
for (slots_checked = 0; slots_checked < pool->area_nslabs; ) {
1039+
phys_addr_t tlb_addr;
1040+
10401041
slot_index = slot_base + index;
1042+
tlb_addr = slot_addr(tbl_dma_addr, slot_index);
10411043

1042-
if (orig_addr &&
1043-
(slot_addr(tbl_dma_addr, slot_index) &
1044-
iotlb_align_mask) != (orig_addr & iotlb_align_mask)) {
1044+
if ((tlb_addr & alloc_align_mask) ||
1045+
(orig_addr && (tlb_addr & iotlb_align_mask) !=
1046+
(orig_addr & iotlb_align_mask))) {
10451047
index = wrap_area_index(pool, index + 1);
10461048
slots_checked++;
10471049
continue;

0 commit comments

Comments
 (0)