| From 885decec9237c9cc85a337605e478efbb54f5cc7 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 8 Mar 2024 15:28:24 +0000 |
| Subject: swiotlb: Fix double-allocation of slots due to broken alignment |
| handling |
| |
| From: Will Deacon <will@kernel.org> |
| |
| [ Upstream commit 04867a7a33324c9c562ee7949dbcaab7aaad1fb4 ] |
| |
| Commit bbb73a103fbb ("swiotlb: fix a braino in the alignment check fix"), |
| which was a fix for commit 0eee5ae10256 ("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: bbb73a103fbb ("swiotlb: fix a braino in the alignment check fix") |
| Fixes: 0eee5ae10256 ("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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/dma/swiotlb.c | 26 ++++++++++++++------------ |
| 1 file changed, 14 insertions(+), 12 deletions(-) |
| |
| diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c |
| index 33d942615be54..8fba61069b84d 100644 |
| --- a/kernel/dma/swiotlb.c |
| +++ b/kernel/dma/swiotlb.c |
| @@ -982,7 +982,7 @@ static int swiotlb_area_find_slots(struct device *dev, struct io_tlb_pool *pool, |
| phys_to_dma_unencrypted(dev, pool->start) & boundary_mask; |
| unsigned long max_slots = get_max_slots(boundary_mask); |
| unsigned int iotlb_align_mask = |
| - dma_get_min_align_mask(dev) | alloc_align_mask; |
| + dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1); |
| unsigned int nslots = nr_slots(alloc_size), stride; |
| unsigned int offset = swiotlb_align_offset(dev, orig_addr); |
| unsigned int index, slots_checked, count = 0, i; |
| @@ -994,18 +994,17 @@ static int swiotlb_area_find_slots(struct device *dev, struct io_tlb_pool *pool, |
| BUG_ON(area_index >= pool->nareas); |
| |
| /* |
| - * For allocations of PAGE_SIZE or larger only look for page aligned |
| - * allocations. |
| + * For mappings with an alignment requirement don't bother looping to |
| + * unaligned slots once we found an aligned one. |
| */ |
| - if (alloc_size >= PAGE_SIZE) |
| - iotlb_align_mask |= ~PAGE_MASK; |
| - iotlb_align_mask &= ~(IO_TLB_SIZE - 1); |
| + stride = get_max_slots(max(alloc_align_mask, iotlb_align_mask)); |
| |
| /* |
| - * For mappings with an alignment requirement don't bother looping to |
| - * unaligned slots once we found an aligned one. |
| + * For allocations of PAGE_SIZE or larger only look for page aligned |
| + * allocations. |
| */ |
| - stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1; |
| + if (alloc_size >= PAGE_SIZE) |
| + stride = umax(stride, PAGE_SHIFT - IO_TLB_SHIFT + 1); |
| |
| spin_lock_irqsave(&area->lock, flags); |
| if (unlikely(nslots > pool->area_nslabs - area->used)) |
| @@ -1015,11 +1014,14 @@ static int swiotlb_area_find_slots(struct device *dev, struct io_tlb_pool *pool, |
| index = area->index; |
| |
| for (slots_checked = 0; slots_checked < pool->area_nslabs; ) { |
| + phys_addr_t tlb_addr; |
| + |
| slot_index = slot_base + index; |
| + tlb_addr = slot_addr(tbl_dma_addr, slot_index); |
| |
| - if (orig_addr && |
| - (slot_addr(tbl_dma_addr, slot_index) & |
| - iotlb_align_mask) != (orig_addr & iotlb_align_mask)) { |
| + if ((tlb_addr & alloc_align_mask) || |
| + (orig_addr && (tlb_addr & iotlb_align_mask) != |
| + (orig_addr & iotlb_align_mask))) { |
| index = wrap_area_index(pool, index + 1); |
| slots_checked++; |
| continue; |
| -- |
| 2.43.0 |
| |