linux-kernel - Re: KMSAN: uninit-value in eth_type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0d5f2179-b89b-42b6-805d-6a50d2268eca@samsung.com>
Date: Mon, 13 Oct 2025 09:31:08 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Alexander Potapenko <glider@...gle.com>, Robin Murphy
	<robin.murphy@....com>, Christoph Hellwig <hch@...radead.org>, Leon
	Romanovsky <leonro@...dia.com>, mhklinux@...look.com
Cc: anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com, Aleksandr
	Nogikh <nogikh@...gle.com>
Subject: Re: KMSAN: uninit-value in eth_type_trans

On 08.10.2025 10:43, Alexander Potapenko wrote:
> On Tue, Oct 7, 2025 at 8:51 AM Alexander Potapenko <glider@...gle.com> 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://protect2.fireeye.com/v1/url?k=837a6d67-dce15478-837be628-000babdfecba-aa25926458f9fd30&q=1&e=a3963b1b-328b-4f69-8ca5-ffd6fc777dd7&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv6.17.1%2Fsource%2Fkernel%2Fdma%2Fswiotlb.c%23L1431),
> 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().

Well, I didn't consider swiotlb a special case so far. I did a simple 
test with my PoC patch mentioned earlier in this thread with 
'swiotlb=force' kernel parameter and I didn't observe any kmsan issues, 
but I admin that this wasn't exhaustive test.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland