You are right, thanks. Since 41a6328b2c55 ("hv_netvsc: Preserve
contiguous PFN grouping in the page buffer array") a pb entry
describes a full physically contiguous fragment and pb[i].len can
exceed PAGE_SIZE, while kmap_local_page() maps a single page. v3
copies page by page, splitting at native page boundaries, and was
posted as a new thread per netdev process.чт, 4 июн. 2026 г. в 19:39, <[email protected]>: > > 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 <[email protected]> > > 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/[email protected]?part=1
