On Tue, Oct 7, 2025 at 8:51 AM Alexander Potapenko <[email protected]> wrote:
> Folks, as far as I understand, dma_direct_sync_single_for_cpu() and
> dma_direct_sync_single_for_device() are the places where we send data
> to or from the device.
> Should we add KMSAN annotations to those functions to catch infoleaks
> and mark data from devices as initialized?
Something along the lines of:
======================================
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 0d37da3d95b65..7f59de19c1c87 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -42,6 +42,7 @@
#include <linux/string.h>
#include <linux/swiotlb.h>
#include <linux/types.h>
+#include <linux/kmsan-checks.h>
#ifdef CONFIG_DMA_RESTRICTED_POOL
#include <linux/of.h>
#include <linux/of_fdt.h>
@@ -903,10 +904,13 @@ static void swiotlb_bounce(struct device *dev,
phys_addr_t tlb_addr, size_t size
local_irq_save(flags);
page = pfn_to_page(pfn);
- if (dir == DMA_TO_DEVICE)
+ if (dir == DMA_TO_DEVICE) {
+ kmsan_check_highmem_page(page, offset, sz);
memcpy_from_page(vaddr, page, offset, sz);
- else
+ } else {
+ kmsan_unpoison_memory(vaddr, sz);
memcpy_to_page(page, offset, vaddr, sz);
+ }
local_irq_restore(flags);
size -= sz;
@@ -915,8 +919,10 @@ static void swiotlb_bounce(struct device *dev,
phys_addr_t tlb_addr, size_t size
offset = 0;
}
} else if (dir == DMA_TO_DEVICE) {
+ kmsan_check_memory(phys_to_virt(orig_addr), size);
memcpy(vaddr, phys_to_virt(orig_addr), size);
} else {
+ kmsan_unpoison_memory(vaddr, size);
memcpy(phys_to_virt(orig_addr), vaddr, size);
}
}
======================================
should be conceptually right, but according to the comment in
swiotlb_tbl_map_single()
(https://elixir.bootlin.com/linux/v6.17.1/source/kernel/dma/swiotlb.c#L1431),
that function is deliberately copying the buffer to the device, even
when it is uninitialized - and KMSAN actually started reporting that
when I applied the above patch.
How should we handle this case?
Not adding the kmsan_check_memory() calls will solve the problem, but
there might be real infoleaks that we won't detect.
We could unpoison the buffer before passing it to
swiotlb_tbl_map_single() to ignore just the first infoleak on the
buffer.
Alternatively, we could require callers to always initialize the
buffer passed to swiotlb_tbl_map_single().
