* [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