* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.