Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] ipv6: account for fraggap on the paged allocation path
@ 2026-06-09 15:32 Wongi Lee
  2026-06-11 10:23 ` Ido Schimmel
  0 siblings, 1 reply; 3+ messages in thread
From: Wongi Lee @ 2026-06-09 15:32 UTC (permalink / raw)
  To: netdev
  Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman

In __ip6_append_data(), when the paged-allocation branch is taken
(MSG_MORE / NETIF_F_SG / large fraglen), alloclen and pagedlen are
computed as

	alloclen = fragheaderlen + transhdrlen;
	pagedlen = datalen - transhdrlen;

datalen already includes fraggap (datalen = length + fraggap), but
the fraggap bytes carried over from the previous skb are copied into
the new skb's linear area at offset transhdrlen by the subsequent
skb_copy_and_csum_bits(). The linear area is therefore undersized by
fraggap bytes while pagedlen is overstated by the same amount, and
the copy writes past skb->end into the trailing skb_shared_info.

An unprivileged user can trigger this via a UDPv6 socket using
MSG_MORE together with MSG_SPLICE_PAGES.

The non-paged branch a few lines above sets
alloclen = fraglen = datalen + fragheaderlen, which already accounts
for fraggap because datalen does. Bring the paged branch in line by
adding fraggap to alloclen and subtracting it from pagedlen.

Fixes: 773ba4fe9104 ("ipv6: avoid partial copy for zc")
Assisted-by: Xint
Signed-off-by: Jungwoo Lee <jwlee2217@gmail.com>
Signed-off-by: Wongi Lee <qw3rtyp0@gmail.com>
---
v2:
- Fix mail format.
- v1: https://lore.kernel.org/netdev/aibiIYMAwUErTw5U@DESKTOP-19IMU7U.localdomain
---
 net/ipv6/ip6_output.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index c14adcdd4396..265502caa44b 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1668,8 +1668,8 @@ static int __ip6_append_data(struct sock *sk,
 				  !(rt->dst.dev->features & NETIF_F_SG)))
 				alloclen = fraglen;
 			else {
-				alloclen = fragheaderlen + transhdrlen;
-				pagedlen = datalen - transhdrlen;
+				alloclen = fragheaderlen + transhdrlen + fraggap;
+				pagedlen = datalen - transhdrlen - fraggap;
 			}
 			alloclen += alloc_extra;
 
-- 
2.34.1

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

* Re: [PATCH net v2] ipv6: account for fraggap on the paged allocation path
  2026-06-09 15:32 [PATCH net v2] ipv6: account for fraggap on the paged allocation path Wongi Lee
@ 2026-06-11 10:23 ` Ido Schimmel
  2026-06-11 13:21   ` Wongi Lee
  0 siblings, 1 reply; 3+ messages in thread
From: Ido Schimmel @ 2026-06-11 10:23 UTC (permalink / raw)
  To: Wongi Lee
  Cc: netdev, David Ahern, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, asml.silence, dhowells,
	willemb

+ Pavel, David, Willem

On Wed, Jun 10, 2026 at 12:32:03AM +0900, Wongi Lee wrote:
> In __ip6_append_data(), when the paged-allocation branch is taken
> (MSG_MORE / NETIF_F_SG / large fraglen), alloclen and pagedlen are
> computed as
> 
> 	alloclen = fragheaderlen + transhdrlen;
> 	pagedlen = datalen - transhdrlen;
> 
> datalen already includes fraggap (datalen = length + fraggap), but
> the fraggap bytes carried over from the previous skb are copied into
> the new skb's linear area at offset transhdrlen by the subsequent
> skb_copy_and_csum_bits(). The linear area is therefore undersized by
> fraggap bytes while pagedlen is overstated by the same amount, and
> the copy writes past skb->end into the trailing skb_shared_info.
> 
> An unprivileged user can trigger this via a UDPv6 socket using
> MSG_MORE together with MSG_SPLICE_PAGES.
> 
> The non-paged branch a few lines above sets
> alloclen = fraglen = datalen + fragheaderlen, which already accounts
> for fraggap because datalen does. Bring the paged branch in line by
> adding fraggap to alloclen and subtracting it from pagedlen.
> 
> Fixes: 773ba4fe9104 ("ipv6: avoid partial copy for zc")

I'm OK with this tag if we want to be defensive, but isn't the data
corruption only trigger-able since commit ce650a166335 ("udp6: Fix
__ip6_append_data()'s handling of MSG_SPLICE_PAGES") ?

AFAICT, before ce650a166335, a negative 'copy' would always result in
EINVAL being returned. I would at least mention this in the commit
message.

Speaking of a negative 'copy', I think Sashiko is correct [1] and the
comment regarding pagedlen>0 is now stale.

Finally, what about IPv4? It has the same code in commit 8eb77cc73977
("ipv4: avoid partial copy for zc").

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/aigx83czv%2BUJZA0d%40DESKTOP-19IMU7U.localdomain

> Assisted-by: Xint
> Signed-off-by: Jungwoo Lee <jwlee2217@gmail.com>
> Signed-off-by: Wongi Lee <qw3rtyp0@gmail.com>
> ---
> v2:
> - Fix mail format.
> - v1: https://lore.kernel.org/netdev/aibiIYMAwUErTw5U@DESKTOP-19IMU7U.localdomain
> ---
>  net/ipv6/ip6_output.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index c14adcdd4396..265502caa44b 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1668,8 +1668,8 @@ static int __ip6_append_data(struct sock *sk,
>  				  !(rt->dst.dev->features & NETIF_F_SG)))
>  				alloclen = fraglen;
>  			else {
> -				alloclen = fragheaderlen + transhdrlen;
> -				pagedlen = datalen - transhdrlen;
> +				alloclen = fragheaderlen + transhdrlen + fraggap;
> +				pagedlen = datalen - transhdrlen - fraggap;
>  			}
>  			alloclen += alloc_extra;
>  
> -- 
> 2.34.1

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

* Re: [PATCH net v2] ipv6: account for fraggap on the paged allocation path
  2026-06-11 10:23 ` Ido Schimmel
@ 2026-06-11 13:21   ` Wongi Lee
  0 siblings, 0 replies; 3+ messages in thread
From: Wongi Lee @ 2026-06-11 13:21 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, David Ahern, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, asml.silence, dhowells,
	willemb, jwlee2217

On Thu, Jun 11, 2026 at 01:23:03PM +0300, Ido Schimmel wrote:
> + Pavel, David, Willem
> 
> On Wed, Jun 10, 2026 at 12:32:03AM +0900, Wongi Lee wrote:
> > In __ip6_append_data(), when the paged-allocation branch is taken
> > (MSG_MORE / NETIF_F_SG / large fraglen), alloclen and pagedlen are
> > computed as
> > 
> > 	alloclen = fragheaderlen + transhdrlen;
> > 	pagedlen = datalen - transhdrlen;
> > 
> > datalen already includes fraggap (datalen = length + fraggap), but
> > the fraggap bytes carried over from the previous skb are copied into
> > the new skb's linear area at offset transhdrlen by the subsequent
> > skb_copy_and_csum_bits(). The linear area is therefore undersized by
> > fraggap bytes while pagedlen is overstated by the same amount, and
> > the copy writes past skb->end into the trailing skb_shared_info.
> > 
> > An unprivileged user can trigger this via a UDPv6 socket using
> > MSG_MORE together with MSG_SPLICE_PAGES.
> > 
> > The non-paged branch a few lines above sets
> > alloclen = fraglen = datalen + fragheaderlen, which already accounts
> > for fraggap because datalen does. Bring the paged branch in line by
> > adding fraggap to alloclen and subtracting it from pagedlen.
> > 
> > Fixes: 773ba4fe9104 ("ipv6: avoid partial copy for zc")
> 
> I'm OK with this tag if we want to be defensive, but isn't the data
> corruption only trigger-able since commit ce650a166335 ("udp6: Fix
> __ip6_append_data()'s handling of MSG_SPLICE_PAGES") ?
> 
> AFAICT, before ce650a166335, a negative 'copy' would always result in
> EINVAL being returned. I would at least mention this in the commit
> message.
>

+ Jungwoo

Right. I will add that info when write v3.

> Speaking of a negative 'copy', I think Sashiko is correct [1] and the
> comment regarding pagedlen>0 is now stale.
> 
> Finally, what about IPv4? It has the same code in commit 8eb77cc73977
> ("ipv4: avoid partial copy for zc").
> 
> [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/aigx83czv%2BUJZA0d%40DESKTOP-19IMU7U.localdomain
> 

I think ipv4 has same problem (but not seems trigger oob to skb_shared_info). 
Remove comment needed also in both.

Preserving the copy > 0 logic seems good for defensive purpose 
so I will just let it.

> > Assisted-by: Xint
> > Signed-off-by: Jungwoo Lee <jwlee2217@gmail.com>
> > Signed-off-by: Wongi Lee <qw3rtyp0@gmail.com>
> > ---
> > v2:
> > - Fix mail format.
> > - v1: https://lore.kernel.org/netdev/aibiIYMAwUErTw5U@DESKTOP-19IMU7U.localdomain
> > ---
> >  net/ipv6/ip6_output.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index c14adcdd4396..265502caa44b 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -1668,8 +1668,8 @@ static int __ip6_append_data(struct sock *sk,
> >  				  !(rt->dst.dev->features & NETIF_F_SG)))
> >  				alloclen = fraglen;
> >  			else {
> > -				alloclen = fragheaderlen + transhdrlen;
> > -				pagedlen = datalen - transhdrlen;
> > +				alloclen = fragheaderlen + transhdrlen + fraggap;
> > +				pagedlen = datalen - transhdrlen - fraggap;
> >  			}
> >  			alloclen += alloc_extra;
> >  
> > -- 
> > 2.34.1

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

end of thread, other threads:[~2026-06-11 13:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 15:32 [PATCH net v2] ipv6: account for fraggap on the paged allocation path Wongi Lee
2026-06-11 10:23 ` Ido Schimmel
2026-06-11 13:21   ` Wongi Lee

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