Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
@ 2026-06-02 15:52 LeantionX
  2026-06-02 22:54 ` [EXTERNAL] " Haiyang Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: LeantionX @ 2026-06-02 15:52 UTC (permalink / raw)
  To: netdev
  Cc: linux-hyperv, kys, haiyangz, wei.liu, decui, longli,
	andrew+netdev, kuba, pabeni, edumazet, davem, stable,
	linux-kernel, Anton Leontev

From: Anton Leontev <leontyevantony@gmail.com>

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).

Map the fragment page on demand with kmap_local_page()/kunmap_local()
instead. Using pfn_to_page() on pb[i].pfn maps exactly the page
described by the page buffer entry. On configurations without HIGHMEM
(amd64, i386 without CONFIG_HIGHMEM) 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>
---
 drivers/net/hyperv/netvsc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 59e95341f9b1..6984f6c97257 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,13 @@ 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);
+		struct page *page = pfn_to_page(pb[i].pfn);
+		char *src = kmap_local_page(page);
 		u32 offset = pb[i].offset;
 		u32 len = pb[i].len;
 
 		memcpy(dest, (src + offset), len);
+		kunmap_local(src);
 		dest += len;
 	}
 
-- 
2.43.0


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

* RE: [EXTERNAL] [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-02 15:52 [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf LeantionX
@ 2026-06-02 22:54 ` Haiyang Zhang
  2026-06-03  5:29   ` Anton Leontev
  2026-06-03 15:53 ` sashiko-bot
  2026-06-03 16:38 ` [PATCH net v2] " Anton Leontev
  2 siblings, 1 reply; 10+ messages in thread
From: Haiyang Zhang @ 2026-06-02 22:54 UTC (permalink / raw)
  To: LeantionX, netdev@vger.kernel.org
  Cc: linux-hyperv@vger.kernel.org, KY Srinivasan, wei.liu@kernel.org,
	Dexuan Cui, Long Li, andrew+netdev@lunn.ch, kuba@kernel.org,
	pabeni@redhat.com, edumazet@google.com, davem@davemloft.net,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: LeantionX <leontyevantony@gmail.com>
> Sent: Tuesday, June 2, 2026 11:52 AM
> To: netdev@vger.kernel.org
> Cc: linux-hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> Haiyang Zhang <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <DECUI@microsoft.com>; Long Li <longli@microsoft.com>;
> andrew+netdev@lunn.ch; kuba@kernel.org; pabeni@redhat.com;
> edumazet@google.com; davem@davemloft.net; stable@vger.kernel.org; linux-
> kernel@vger.kernel.org; Anton Leontev <leontyevantony@gmail.com>
> Subject: [EXTERNAL] [PATCH net] hv_netvsc: use kmap_local_page in
> netvsc_copy_to_send_buf
> 
> [You don't often get email from leontyevantony@gmail.com. Learn why this
> is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> From: Anton Leontev <leontyevantony@gmail.com>
> 
> 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.
Please include the stack trace in patch description.

> 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).
> 
> Map the fragment page on demand with kmap_local_page()/kunmap_local()
> instead. Using pfn_to_page() on pb[i].pfn maps exactly the page
> described by the page buffer entry. On configurations without HIGHMEM
> (amd64, i386 without CONFIG_HIGHMEM) kmap_local_page() reduces to
> page_address(), so this is a no-op there.

So, on 64bit kernel, it has no performance impact?

Thanks,
- Haiyang


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

* Re: [EXTERNAL] [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-02 22:54 ` [EXTERNAL] " Haiyang Zhang
@ 2026-06-03  5:29   ` Anton Leontev
  0 siblings, 0 replies; 10+ messages in thread
From: Anton Leontev @ 2026-06-03  5:29 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: netdev@vger.kernel.org, linux-hyperv@vger.kernel.org,
	KY Srinivasan, wei.liu@kernel.org, Dexuan Cui, Long Li,
	andrew+netdev@lunn.ch, kuba@kernel.org, pabeni@redhat.com,
	edumazet@google.com, davem@davemloft.net, stable@vger.kernel.org,
	linux-kernel@vger.kernel.org

>
>
>
> > -----Original Message-----
> > From: LeantionX <leontyevantony@gmail.com>
> > Sent: Tuesday, June 2, 2026 11:52 AM
> > To: netdev@vger.kernel.org
> > Cc: linux-hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>;
> > Haiyang Zhang <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> > <DECUI@microsoft.com>; Long Li <longli@microsoft.com>;
> > andrew+netdev@lunn.ch; kuba@kernel.org; pabeni@redhat.com;
> > edumazet@google.com; davem@davemloft.net; stable@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Anton Leontev <leontyevantony@gmail.com>
> > Subject: [EXTERNAL] [PATCH net] hv_netvsc: use kmap_local_page in
> > netvsc_copy_to_send_buf
> >
> > [You don't often get email from leontyevantony@gmail.com. Learn why this
> > is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Anton Leontev <leontyevantony@gmail.com>
> >
> > 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.
> Please include the stack trace in patch description.
>
> > 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).
> >
> > Map the fragment page on demand with kmap_local_page()/kunmap_local()
> > instead. Using pfn_to_page() on pb[i].pfn maps exactly the page
> > described by the page buffer entry. On configurations without HIGHMEM
> > (amd64, i386 without CONFIG_HIGHMEM) kmap_local_page() reduces to
> > page_address(), so this is a no-op there.
>
> So, on 64bit kernel, it has no performance impact?
>
> Thanks,
> - Haiyang
>

Correct. On 64-bit (and any !CONFIG_HIGHMEM config) all pages are
permanently present in the kernel direct map, so kmap_local_page()
folds to page_address() and kunmap_local() is a no-op. The generated
code is therefore equivalent to the previous direct-map access, with
no extra mapping cost on the tx path.

The kmap is only meaningful on 32-bit CONFIG_HIGHMEM, where the
fragment page may live above the LOWMEM boundary and the old
phys_to_virt() result is invalid.

Thanks,
Anton

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

* Re: [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-02 15:52 [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf LeantionX
  2026-06-02 22:54 ` [EXTERNAL] " Haiyang Zhang
@ 2026-06-03 15:53 ` sashiko-bot
  2026-06-03 16:29   ` Anton Leontev
  2026-06-03 16:38 ` [PATCH net v2] " Anton Leontev
  2 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-06-03 15:53 UTC (permalink / raw)
  To: LeantionX; +Cc: linux-hyperv

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Passing a Hyper-V PFN directly to `pfn_to_page()` causes out-of-bounds memory accesses on architectures where `PAGE_SIZE > 4KB` (e.g., ARM64 with 64KB pages), and loses the intra-page chunk offset.
--

commit a844fbf018cd37eedc9a1f0aea79d6235c00bec3
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() in
netvsc_copy_to_send_buf() to map skb fragment pages. This is intended to
fix a fatal transmit softirq fault on 32-bit x86 configurations with
HIGHMEM, where phys_to_virt() is only valid for LOWMEM addresses.

> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..6984f6c97257d 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c

[ ... ]

> @@ -965,11 +966,13 @@ 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);
> +		struct page *page = pfn_to_page(pb[i].pfn);
> +		char *src = kmap_local_page(page);

[Severity: Critical]
Does this cause out-of-bounds memory accesses on architectures with a native
page size greater than 4KB?

The pb[i].pfn field contains a Hyper-V PFN, which always operates at 4KB
granularity. Passing this directly to pfn_to_page() assumes it is a native
Linux PFN.

On architectures where the native page size is larger than 4KB (such as
ARM64 with 64KB pages), the Hyper-V PFN is significantly larger than the
corresponding native Linux PFN. Using it with pfn_to_page() would return an
invalid struct page pointer.

Calling kmap_local_page() on this invalid pointer in
netvsc_copy_to_send_buf() and subsequently reading from it could lead to
memory corruption, out-of-bounds accesses, or kernel panics.

Could this also lose the native page offset of the original 4KB chunk within
the larger Linux page, resulting in an incorrect read offset during the
subsequent memcpy?

>  		u32 offset = pb[i].offset;
>  		u32 len = pb[i].len;
>  
>  		memcpy(dest, (src + offset), len);
> +		kunmap_local(src);
>  		dest += len;
>  	}

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

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

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

You are right, thanks for catching this. pb[i].pfn is a Hyper-V PFN at
4K granularity, so passing it directly to pfn_to_page() is wrong where
PAGE_SIZE > 4K, and it also drops the intra-page offset. Fixed in v2 by
reconstructing the physical address and using phys_to_page() +
offset_in_page(). Built for i386 (HIGHMEM) and arm64 (64K pages).

Thanks,
Anton

ср, 3 июн. 2026 г. в 18:53, <sashiko-bot@kernel.org>:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Passing a Hyper-V PFN directly to `pfn_to_page()` causes out-of-bounds memory accesses on architectures where `PAGE_SIZE > 4KB` (e.g., ARM64 with 64KB pages), and loses the intra-page chunk offset.
> --
>
> commit a844fbf018cd37eedc9a1f0aea79d6235c00bec3
> 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() in
> netvsc_copy_to_send_buf() to map skb fragment pages. This is intended to
> fix a fatal transmit softirq fault on 32-bit x86 configurations with
> HIGHMEM, where phys_to_virt() is only valid for LOWMEM addresses.
>
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> > index 59e95341f9b1e..6984f6c97257d 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
>
> [ ... ]
>
> > @@ -965,11 +966,13 @@ 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);
> > +             struct page *page = pfn_to_page(pb[i].pfn);
> > +             char *src = kmap_local_page(page);
>
> [Severity: Critical]
> Does this cause out-of-bounds memory accesses on architectures with a native
> page size greater than 4KB?
>
> The pb[i].pfn field contains a Hyper-V PFN, which always operates at 4KB
> granularity. Passing this directly to pfn_to_page() assumes it is a native
> Linux PFN.
>
> On architectures where the native page size is larger than 4KB (such as
> ARM64 with 64KB pages), the Hyper-V PFN is significantly larger than the
> corresponding native Linux PFN. Using it with pfn_to_page() would return an
> invalid struct page pointer.
>
> Calling kmap_local_page() on this invalid pointer in
> netvsc_copy_to_send_buf() and subsequently reading from it could lead to
> memory corruption, out-of-bounds accesses, or kernel panics.
>
> Could this also lose the native page offset of the original 4KB chunk within
> the larger Linux page, resulting in an incorrect read offset during the
> subsequent memcpy?
>
> >               u32 offset = pb[i].offset;
> >               u32 len = pb[i].len;
> >
> >               memcpy(dest, (src + offset), len);
> > +             kunmap_local(src);
> >               dest += len;
> >       }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260602155210.90987-1-leontyevanton1995@gmail.com?part=1

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

* [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
  2026-06-02 15:52 [PATCH net] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf LeantionX
  2026-06-02 22:54 ` [EXTERNAL] " Haiyang Zhang
  2026-06-03 15:53 ` sashiko-bot
@ 2026-06-03 16:38 ` Anton Leontev
  2026-06-04 16:39   ` sashiko-bot
  2 siblings, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread

* Re: [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, 0 replies; 10+ 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] 10+ messages in thread

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

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

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