Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-02 15:52 [PATCH net] " LeantionX
@ 2026-06-03 16:38 ` Anton Leontev
  2026-06-04 16:39   ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Anton Leontev @ 2026-06-03 16:38 UTC (permalink / raw)
  To: netdev
  Cc: linux-hyperv, haiyangz, kys, wei.liu, decui, longli, kuba, pabeni,
	edumazet, davem, stable, linux-kernel, Anton Leontev

netvsc_copy_to_send_buf() copies skb fragment pages into the shared
VMBus send buffer using phys_to_virt() on the fragment PFN. On 32-bit
x86 with CONFIG_HIGHMEM=y, phys_to_virt() (i.e. __va()) is only valid
for LOWMEM addresses below 896 MiB. For a HIGHMEM page it returns an
address that has no kernel page table entry and lies outside the
kernel direct map, so the subsequent memcpy() faults. As this happens
on the transmit softirq path, the fault is fatal.

A HIGHMEM fragment reaches this path whenever the page backing an skb
fragment lives above the LOWMEM boundary, which is common on a 32-bit
guest with several GiB of RAM (for example when the in-kernel NFS
server splices page cache pages directly into the reply skb).

pb[i].pfn is a Hyper-V PFN at HV_HYP_PAGE_SIZE (4K) granularity. The
physical address is reconstructed first and phys_to_page() is used to
obtain the native struct page, with offset_in_page() added so the
in-page offset stays correct where PAGE_SIZE > HV_HYP_PAGE_SIZE (e.g.
arm64 with 64K pages). The page is then mapped on demand with
kmap_local_page()/kunmap_local(). On !CONFIG_HIGHMEM configs
kmap_local_page() reduces to page_address(), so this is a no-op there.

Fixes: c25aaf814a63 ("hyperv: Enable sendbuf mechanism on the send path")
Cc: stable@vger.kernel.org
Signed-off-by: Anton Leontev <leontyevantony@gmail.com>
---
v2:
 - Reconstruct the physical address from the Hyper-V PFN and use
   phys_to_page() + offset_in_page() instead of pfn_to_page() on the
   raw PFN, correct where PAGE_SIZE > 4K (e.g. arm64 64K pages).
   Reported by Haiyang Zhang.
 - Built for i386 (CONFIG_HIGHMEM) and arm64 (64K pages).
 drivers/net/hyperv/netvsc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 59e95341f9b1..2038d9f5c9f9 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -12,6 +12,7 @@
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/mm.h>
+#include <linux/highmem.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
@@ -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);
+		kunmap_local(src);
 		dest += len;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
@ 2026-06-03 17:25 Anton Leontev
  2026-06-04 17:25 ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Anton Leontev @ 2026-06-03 17:25 UTC (permalink / raw)
  To: netdev
  Cc: linux-hyperv, haiyangz, kys, wei.liu, decui, longli, kuba, pabeni,
	edumazet, davem, stable, linux-kernel, Anton Leontev

netvsc_copy_to_send_buf() copies skb fragment pages into the shared
VMBus send buffer using phys_to_virt() on the fragment PFN. On 32-bit
x86 with CONFIG_HIGHMEM=y, phys_to_virt() (i.e. __va()) is only valid
for LOWMEM addresses below 896 MiB. For a HIGHMEM page it returns an
address that has no kernel page table entry and lies outside the
kernel direct map, so the subsequent memcpy() faults. As this happens
on the transmit softirq path, the fault is fatal.

A HIGHMEM fragment reaches this path whenever the page backing an skb
fragment lives above the LOWMEM boundary, which is common on a 32-bit
guest with several GiB of RAM (for example when the in-kernel NFS
server splices page cache pages directly into the reply skb).

pb[i].pfn is a Hyper-V PFN at HV_HYP_PAGE_SIZE (4K) granularity. The
physical address is reconstructed first and phys_to_page() is used to
obtain the native struct page, with offset_in_page() added so the
in-page offset stays correct where PAGE_SIZE > HV_HYP_PAGE_SIZE (e.g.
arm64 with 64K pages). The page is then mapped on demand with
kmap_local_page()/kunmap_local(). On !CONFIG_HIGHMEM configs
kmap_local_page() reduces to page_address(), so this is a no-op there.

Fixes: c25aaf814a63 ("hyperv: Enable sendbuf mechanism on the send path")
Cc: stable@vger.kernel.org
Signed-off-by: Anton Leontev <leontyevantony@gmail.com>
---
v2:
 - Reconstruct the physical address from the Hyper-V PFN and use
   phys_to_page() + offset_in_page() instead of pfn_to_page() on the
   raw PFN, correct where PAGE_SIZE > 4K (e.g. arm64 64K pages).
   Reported by Haiyang Zhang.
 - Built for i386 (CONFIG_HIGHMEM) and arm64 (64K pages).
 drivers/net/hyperv/netvsc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 59e95341f9b1..2038d9f5c9f9 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -12,6 +12,7 @@
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/mm.h>
+#include <linux/highmem.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/slab.h>
@@ -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);
+		kunmap_local(src);
 		dest += len;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-03 16:38 ` [PATCH net v2] " Anton Leontev
@ 2026-06-04 16:39   ` sashiko-bot
  2026-06-04 16:59     ` Anton Leontev
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-06-04 16:39 UTC (permalink / raw)
  To: Anton Leontev; +Cc: linux-hyperv

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-04 16:39   ` sashiko-bot
@ 2026-06-04 16:59     ` Anton Leontev
  0 siblings, 0 replies; 5+ messages in thread
From: Anton Leontev @ 2026-06-04 16:59 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-hyperv

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, <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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-03 17:25 [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf Anton Leontev
@ 2026-06-04 17:25 ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-04 17:25 UTC (permalink / raw)
  To: Anton Leontev; +Cc: linux-hyperv

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] `memcpy()` overruns the single-page mapping created by `kmap_local_page()` when an SKB fragment spans multiple pages.

Pre-existing issues:
- [High] Concurrent writers to the per-channel `tx_stats.syncp` will corrupt the statistics seqcount, deadlocking readers on 32-bit systems.
--

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

hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf

This commit replaces phys_to_virt() with kmap_local_page() to safely access
highmem SKB fragments on 32-bit systems, preventing page faults.

> 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]
Does this memcpy() risk overrunning the single-page mapping?

kmap_local_page() creates a temporary thread-local virtual mapping of exactly
one native page (4096 bytes on x86). However, the length of the fragment (len)
can exceed a single page. 

For example, init_page_array() maps the entire linear skb->data into a single
hv_page_buffer entry, and skb_headlen() can exceed 4096 bytes with jumbo
frames or GRO/TSO packets.

If pb[i].len + offset > PAGE_SIZE, this will read beyond the mapped 4096-byte
window and access unmapped fixmap slots, potentially triggering a page fault
on the transmit softirq path.

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

[Severity: High]
This is a pre-existing issue, but can concurrent writers to the per-channel
tx_stats.syncp corrupt the statistics seqcount?

Looking at netvsc_ndoxdp_xmit() in drivers/net/hyperv/netvsc_bpf.c, it selects
a transmit queue using q_idx = smp_processor_id() % queues and updates the stats:

drivers/net/hyperv/netvsc_bpf.c:netvsc_ndoxdp_xmit() {
    ...
    tx_stats = &nvsc_dev->chan_table[q_idx].tx_stats;
    u64_stats_update_begin(&tx_stats->syncp);
    tx_stats->xdp_xmit += count;
    u64_stats_update_end(&tx_stats->syncp);
    ...
}

If num_online_cpus() > real_num_tx_queues, multiple CPUs can simultaneously
execute XDP transmit for the same queue.

Additionally, the NAPI TX completion path independently updates the same
syncp on the CPU bound to the VMBus interrupt:

drivers/net/hyperv/netvsc.c:netvsc_send_tx_complete() {
    ...
    tx_stats = &net_device->chan_table[q_idx].tx_stats;
    u64_stats_update_begin(&tx_stats->syncp);
    tx_stats->packets += packet->total_packets;
    tx_stats->bytes += packet->total_bytes;
    u64_stats_update_end(&tx_stats->syncp);
    ...
}

Since u64_stats_update_begin() does not use locks, concurrent updates could
leave the seqcount odd forever. This would cause readers like ndo_get_stats64()
to spin infinitely in u64_stats_fetch_retry(), deadlocking the CPU on 32-bit
systems.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-04 17:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03 17:25 [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf Anton Leontev
2026-06-04 17:25 ` sashiko-bot
  -- strict thread matches above, loose matches on Subject: below --
2026-06-02 15:52 [PATCH net] " LeantionX
2026-06-03 16:38 ` [PATCH net v2] " Anton Leontev
2026-06-04 16:39   ` sashiko-bot
2026-06-04 16:59     ` Anton Leontev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox