Skip to content

Commit 3cf5abf

Browse files
PlaidCatPlaidCat
committed
swiotlb: Fix double-allocation of slots due to broken alignment handling
jira LE-1907 cve CVE-2024-35814 Rebuild_History Non-Buildable kernel-4.18.0-553.16.1.el8_10 commit-author Will Deacon <[email protected]> commit 04867a7 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-4.18.0-553.16.1.el8_10/04867a7a.failed 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 <[email protected]> Reviewed-by: Michael Kelley <[email protected]> Reviewed-by: Petr Tesarik <[email protected]> Tested-by: Nicolin Chen <[email protected]> Tested-by: Michael Kelley <[email protected]> Signed-off-by: Christoph Hellwig <[email protected]> (cherry picked from commit 04867a7) Signed-off-by: Jonathan Maple <[email protected]> # Conflicts: # kernel/dma/swiotlb.c
1 parent 55a9012 commit 3cf5abf

File tree

1 file changed

+215
-0
lines changed

1 file changed

+215
-0
lines changed

‎ciq/ciq_backports/kernel-4.18.0-553.16.1.el8_10/04867a7a.failed‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
swiotlb: Fix double-allocation of slots due to broken alignment handling
2+
3+
jira LE-1907
4+
cve CVE-2024-35814
5+
Rebuild_History Non-Buildable kernel-4.18.0-553.16.1.el8_10
6+
commit-author Will Deacon <[email protected]>
7+
commit 04867a7a33324c9c562ee7949dbcaab7aaad1fb4
8+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
9+
Will be included in final tarball splat. Ref for failed cherry-pick at:
10+
ciq/ciq_backports/kernel-4.18.0-553.16.1.el8_10/04867a7a.failed
11+
12+
Commit bbb73a103fbb ("swiotlb: fix a braino in the alignment check fix"),
13+
which was a fix for commit 0eee5ae10256 ("swiotlb: fix slot alignment
14+
checks"), causes a functional regression with vsock in a virtual machine
15+
using bouncing via a restricted DMA SWIOTLB pool.
16+
17+
When virtio allocates the virtqueues for the vsock device using
18+
dma_alloc_coherent(), the SWIOTLB search can return page-unaligned
19+
allocations if 'area->index' was left unaligned by a previous allocation
20+
from the buffer:
21+
22+
# Final address in brackets is the SWIOTLB address returned to the caller
23+
| virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1645-1649/7168 (0x98326800)
24+
| virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1649-1653/7168 (0x98328800)
25+
| virtio-pci 0000:00:07.0: orig_addr 0x0 alloc_size 0x2000, iotlb_align_mask 0x800 stride 0x2: got slot 1653-1657/7168 (0x9832a800)
26+
27+
This ends badly (typically buffer corruption and/or a hang) because
28+
swiotlb_alloc() is expecting a page-aligned allocation and so blindly
29+
returns a pointer to the 'struct page' corresponding to the allocation,
30+
therefore double-allocating the first half (2KiB slot) of the 4KiB page.
31+
32+
Fix the problem by treating the allocation alignment separately to any
33+
additional alignment requirements from the device, using the maximum
34+
of the two as the stride to search the buffer slots and taking care
35+
to ensure a minimum of page-alignment for buffers larger than a page.
36+
37+
This also resolves swiotlb allocation failures occuring due to the
38+
inclusion of ~PAGE_MASK in 'iotlb_align_mask' for large allocations and
39+
resulting in alignment requirements exceeding swiotlb_max_mapping_size().
40+
41+
Fixes: bbb73a103fbb ("swiotlb: fix a braino in the alignment check fix")
42+
Fixes: 0eee5ae10256 ("swiotlb: fix slot alignment checks")
43+
Signed-off-by: Will Deacon <[email protected]>
44+
Reviewed-by: Michael Kelley <[email protected]>
45+
Reviewed-by: Petr Tesarik <[email protected]>
46+
Tested-by: Nicolin Chen <[email protected]>
47+
Tested-by: Michael Kelley <[email protected]>
48+
Signed-off-by: Christoph Hellwig <[email protected]>
49+
(cherry picked from commit 04867a7a33324c9c562ee7949dbcaab7aaad1fb4)
50+
Signed-off-by: Jonathan Maple <[email protected]>
51+
52+
# Conflicts:
53+
# kernel/dma/swiotlb.c
54+
diff --cc kernel/dma/swiotlb.c
55+
index c0e227dcb45e,980a7ec70418..000000000000
56+
--- a/kernel/dma/swiotlb.c
57+
+++ b/kernel/dma/swiotlb.c
58+
@@@ -588,21 -923,88 +588,21 @@@ static unsigned int wrap_area_index(str
59+
}
60+
61+
/*
62+
- * Track the total used slots with a global atomic value in order to have
63+
- * correct information to determine the high water mark. The mem_used()
64+
- * function gives imprecise results because there's no locking across
65+
- * multiple areas.
66+
- */
67+
-#ifdef CONFIG_DEBUG_FS
68+
-static void inc_used_and_hiwater(struct io_tlb_mem *mem, unsigned int nslots)
69+
-{
70+
- unsigned long old_hiwater, new_used;
71+
-
72+
- new_used = atomic_long_add_return(nslots, &mem->total_used);
73+
- old_hiwater = atomic_long_read(&mem->used_hiwater);
74+
- do {
75+
- if (new_used <= old_hiwater)
76+
- break;
77+
- } while (!atomic_long_try_cmpxchg(&mem->used_hiwater,
78+
- &old_hiwater, new_used));
79+
-}
80+
-
81+
-static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
82+
-{
83+
- atomic_long_sub(nslots, &mem->total_used);
84+
-}
85+
-
86+
-#else /* !CONFIG_DEBUG_FS */
87+
-static void inc_used_and_hiwater(struct io_tlb_mem *mem, unsigned int nslots)
88+
-{
89+
-}
90+
-static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
91+
-{
92+
-}
93+
-#endif /* CONFIG_DEBUG_FS */
94+
-
95+
-#ifdef CONFIG_SWIOTLB_DYNAMIC
96+
-#ifdef CONFIG_DEBUG_FS
97+
-static void inc_transient_used(struct io_tlb_mem *mem, unsigned int nslots)
98+
-{
99+
- atomic_long_add(nslots, &mem->transient_nslabs);
100+
-}
101+
-
102+
-static void dec_transient_used(struct io_tlb_mem *mem, unsigned int nslots)
103+
-{
104+
- atomic_long_sub(nslots, &mem->transient_nslabs);
105+
-}
106+
-
107+
-#else /* !CONFIG_DEBUG_FS */
108+
-static void inc_transient_used(struct io_tlb_mem *mem, unsigned int nslots)
109+
-{
110+
-}
111+
-static void dec_transient_used(struct io_tlb_mem *mem, unsigned int nslots)
112+
-{
113+
-}
114+
-#endif /* CONFIG_DEBUG_FS */
115+
-#endif /* CONFIG_SWIOTLB_DYNAMIC */
116+
-
117+
-/**
118+
- * swiotlb_search_pool_area() - search one memory area in one pool
119+
- * @dev: Device which maps the buffer.
120+
- * @pool: Memory pool to be searched.
121+
- * @area_index: Index of the IO TLB memory area to be searched.
122+
- * @orig_addr: Original (non-bounced) IO buffer address.
123+
- * @alloc_size: Total requested size of the bounce buffer,
124+
- * including initial alignment padding.
125+
- * @alloc_align_mask: Required alignment of the allocated buffer.
126+
- *
127+
- * Find a suitable sequence of IO TLB entries for the request and allocate
128+
- * a buffer from the given IO TLB memory area.
129+
- * This function takes care of locking.
130+
- *
131+
- * Return: Index of the first allocated slot, or -1 on error.
132+
+ * Find a suitable number of IO TLB entries size that will fit this request and
133+
+ * allocate a buffer from that IO TLB pool.
134+
*/
135+
-static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool,
136+
- int area_index, phys_addr_t orig_addr, size_t alloc_size,
137+
+static int swiotlb_do_find_slots(struct device *dev, int area_index,
138+
+ phys_addr_t orig_addr, size_t alloc_size,
139+
unsigned int alloc_align_mask)
140+
{
141+
- struct io_tlb_area *area = pool->areas + area_index;
142+
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
143+
+ struct io_tlb_area *area = mem->areas + area_index;
144+
unsigned long boundary_mask = dma_get_seg_boundary(dev);
145+
dma_addr_t tbl_dma_addr =
146+
- phys_to_dma_unencrypted(dev, pool->start) & boundary_mask;
147+
+ phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
148+
unsigned long max_slots = get_max_slots(boundary_mask);
149+
unsigned int iotlb_align_mask =
150+
- dma_get_min_align_mask(dev) | alloc_align_mask;
151+
+ dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
152+
unsigned int nslots = nr_slots(alloc_size), stride;
153+
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
154+
unsigned int index, slots_checked, count = 0, i;
155+
@@@ -611,36 -1013,38 +611,48 @@@
156+
unsigned int slot_index;
157+
158+
BUG_ON(!nslots);
159+
- BUG_ON(area_index >= pool->nareas);
160+
+ BUG_ON(area_index >= mem->nareas);
161+
162+
/*
163+
- * For allocations of PAGE_SIZE or larger only look for page aligned
164+
- * allocations.
165+
+ * For mappings with an alignment requirement don't bother looping to
166+
+ * unaligned slots once we found an aligned one.
167+
*/
168+
- if (alloc_size >= PAGE_SIZE)
169+
- iotlb_align_mask |= ~PAGE_MASK;
170+
- iotlb_align_mask &= ~(IO_TLB_SIZE - 1);
171+
+ stride = get_max_slots(max(alloc_align_mask, iotlb_align_mask));
172+
173+
/*
174+
- * For mappings with an alignment requirement don't bother looping to
175+
- * unaligned slots once we found an aligned one.
176+
+ * For allocations of PAGE_SIZE or larger only look for page aligned
177+
+ * allocations.
178+
*/
179+
- stride = (iotlb_align_mask >> IO_TLB_SHIFT) + 1;
180+
+ if (alloc_size >= PAGE_SIZE)
181+
+ stride = umax(stride, PAGE_SHIFT - IO_TLB_SHIFT + 1);
182+
183+
spin_lock_irqsave(&area->lock, flags);
184+
- if (unlikely(nslots > pool->area_nslabs - area->used))
185+
+ if (unlikely(nslots > mem->area_nslabs - area->used))
186+
goto not_found;
187+
188+
- slot_base = area_index * pool->area_nslabs;
189+
+ slot_base = area_index * mem->area_nslabs;
190+
index = area->index;
191+
192+
++<<<<<<< HEAD
193+
+ for (slots_checked = 0; slots_checked < mem->area_nslabs; ) {
194+
+ slot_index = slot_base + index;
195+
+
196+
+ if (orig_addr &&
197+
+ (slot_addr(tbl_dma_addr, slot_index) &
198+
+ iotlb_align_mask) != (orig_addr & iotlb_align_mask)) {
199+
+ index = wrap_area_index(mem, index + 1);
200+
++=======
201+
+ for (slots_checked = 0; slots_checked < pool->area_nslabs; ) {
202+
+ phys_addr_t tlb_addr;
203+
+
204+
+ slot_index = slot_base + index;
205+
+ tlb_addr = slot_addr(tbl_dma_addr, slot_index);
206+
+
207+
+ if ((tlb_addr & alloc_align_mask) ||
208+
+ (orig_addr && (tlb_addr & iotlb_align_mask) !=
209+
+ (orig_addr & iotlb_align_mask))) {
210+
+ index = wrap_area_index(pool, index + 1);
211+
++>>>>>>> 04867a7a3332 (swiotlb: Fix double-allocation of slots due to broken alignment handling)
212+
slots_checked++;
213+
continue;
214+
}
215+
* Unmerged path kernel/dma/swiotlb.c

0 commit comments

Comments
 (0)