Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch replaces `phys_to_virt()` with `kmap_local_page()` in `netvsc_copy_to_send_buf()` to support HIGHMEM SKB fragments. However, `kmap_local_page()` maps exactly one page (e.g., 4096 bytes). SKB fragments (such as those from TCP sends using `sk_page_frag()` or NFS) can be backed by compound pages where the fragment size (`len`) exceeds a single page, or where `offset + len` crosses a page boundary. The `pb` array populated by `init_page_array()` contains exactly one entry per fragment, meaning `pb[i].len` can exceed `PAGE_SIZE`. When `memcpy(dest, src + offset, len)` is called, it will read past the 4KB window mapped by `kmap_local_page()`. On 32-bit HIGHMEM systems, this accesses adjacent unmapped or unrelated fixmap slots, causing a fatal page fault or memory leakage.
--

commit dfe36312b193784583b28a894f895984d7d0ebba
Author: Anton Leontev <leontyevantony@gmail.com>

hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf

This patch replaces phys_to_virt() with kmap_local_page() to support HIGHMEM
SKB fragments that are beyond the LOWMEM boundary on 32-bit systems.

> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..2038d9f5c9f9a 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
[ ... ]
> @@ -965,11 +966,14 @@ static void netvsc_copy_to_send_buf(struct netvsc_device *net_device,
>  	}
>  
>  	for (i = 0; i < page_count; i++) {
> -		char *src = phys_to_virt(pb[i].pfn << HV_HYP_PAGE_SHIFT);
> -		u32 offset = pb[i].offset;
> +		phys_addr_t paddr = pb[i].pfn << HV_HYP_PAGE_SHIFT;
> +		struct page *page = phys_to_page(paddr);
> +		u32 offset = offset_in_page(paddr) + pb[i].offset;
>  		u32 len = pb[i].len;
> +		char *src = kmap_local_page(page);
>  
>  		memcpy(dest, (src + offset), len);

[Severity: Critical]
Will this memcpy() read beyond the 1-page mapping established by
kmap_local_page()?

If the fragment size (len) plus offset exceeds PAGE_SIZE, it seems memcpy()
will read past the locally mapped window. SKB fragments can be backed by
compound pages where the fragment size exceeds a single page, and
init_page_array() assigns the full fragment size to pb[i].len:

drivers/net/hyperv/netvsc_drv.c:init_page_array() {
	...
	cur_pb->offset = offset_in_hvpage(offset);
	cur_pb->len = skb_frag_size(frag);
	cur_pb->pfn = pfn + (offset >> HV_HYP_PAGE_SHIFT);
	...
}

Other parts of the subsystem also document that a pb entry can cross page
boundaries:

drivers/net/hyperv/netvsc.c:netvsc_build_mpb_array() {
	...
	/* Each entry in the input "pb" array must describe a contiguous range
	 * of guest physical memory so that the pfns are sequential if the range
	 * crosses a page boundary. */
	...
	for (j = 0; j < HVPFN_UP(offset + len); j++)
		mpb_entry->pfnArray[j] = pb[i].pfn + j;
	...
}

Could this regression cause a fatal page fault on 32-bit HIGHMEM guests when
an SKB fragment crosses a page boundary, or leak unrelated fixmap memory if
the adjacent slot happens to be mapped?

> +		kunmap_local(src);
>  		dest += len;
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603163851.18058-1-leontyevantony@gmail.com?part=1