[PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[Sabrina Dubroca]

skb_gro_receive() can currently copy frags between the source and GRO
skb, without checking the zerocopy status, and in particular the
SKBFL_MANAGED_FRAG_REFS flag.

When SKBFL_MANAGED_FRAG_REFS is set, the skb doesn't hold a reference
on the pages in shinfo->frags. Appending those frags to another skb's
frags without fixing up the page refcount can lead to UAF.

When either the last skb in the GRO chain (the one we would append
frags to) or the source skb is zerocopy, skip the frags copy, and just
append the new skb to the frag_list.

This is probably a bit less efficient than calling
skb_zcopy_downgrade_managed(), but then we'd also have to handle the
rest of the zerocopy flags/machinery. This can be improved in
net-next.

Fixes: 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
Reported-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
Assisted-by: Claude:claude-mythos-preview
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/core/gro.c | 3 +++
 1 file changed, 3 insertions(+)

Huzaifa has found this to be exploitable to overwrite the page cache

diff --git a/net/core/gro.c b/net/core/gro.c
index 31d21de5b15a..cae0a0dbfa69 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -123,6 +123,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
 	lp = NAPI_GRO_CB(p)->last;
 	pinfo = skb_shinfo(lp);
 
+	if (skb_zcopy(skb) || skb_zcopy(lp))
+		goto merge;
+
 	if (headlen <= offset) {
 		skb_frag_t *frag;
 		skb_frag_t *frag2;
-- 
2.54.0

Re: [PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[Eric Dumazet]

On Tue, May 19, 2026 at 5:40 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> skb_gro_receive() can currently copy frags between the source and GRO
> skb, without checking the zerocopy status, and in particular the
> SKBFL_MANAGED_FRAG_REFS flag.
>
> When SKBFL_MANAGED_FRAG_REFS is set, the skb doesn't hold a reference
> on the pages in shinfo->frags. Appending those frags to another skb's
> frags without fixing up the page refcount can lead to UAF.
>
> When either the last skb in the GRO chain (the one we would append
> frags to) or the source skb is zerocopy, skip the frags copy, and just
> append the new skb to the frag_list.
>
> This is probably a bit less efficient than calling
> skb_zcopy_downgrade_managed(), but then we'd also have to handle the
> rest of the zerocopy flags/machinery. This can be improved in
> net-next.
>
> Fixes: 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
> Reported-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> Assisted-by: Claude:claude-mythos-preview
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  net/core/gro.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> Huzaifa has found this to be exploitable to overwrite the page cache
>
> diff --git a/net/core/gro.c b/net/core/gro.c
> index 31d21de5b15a..cae0a0dbfa69 100644
> --- a/net/core/gro.c
> +++ b/net/core/gro.c
> @@ -123,6 +123,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
>         lp = NAPI_GRO_CB(p)->last;
>         pinfo = skb_shinfo(lp);
>
> +       if (skb_zcopy(skb) || skb_zcopy(lp))
> +               goto merge;
> +
>         if (headlen <= offset) {
>                 skb_frag_t *frag;
>                 skb_frag_t *frag2;

Do we really want to merge these skbs in the first place?

What about play safe?

diff --git a/net/core/gro.c b/net/core/gro.c
index 31d21de5b15a76439239d15f10d0fd6b6c8ab328..bae89062a053dd9804e772e711e277645e6fee4a
100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -109,6 +109,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
        if (p->pp_recycle != skb->pp_recycle)
                return -ETOOMANYREFS;

+       if (skb_zcopy(p) || skb_zcopy(skb))
+               return -ETOOMANYREFS;
+
        if (unlikely(p->len + len >= netif_get_gro_max_size(p->dev, p) ||
                     NAPI_GRO_CB(skb)->flush))
                return -E2BIG;

Re: [PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[Sabrina Dubroca]

2026-05-19, 05:57:22 -0700, Eric Dumazet wrote:
> On Tue, May 19, 2026 at 5:40 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
> >
> > skb_gro_receive() can currently copy frags between the source and GRO
> > skb, without checking the zerocopy status, and in particular the
> > SKBFL_MANAGED_FRAG_REFS flag.
> >
> > When SKBFL_MANAGED_FRAG_REFS is set, the skb doesn't hold a reference
> > on the pages in shinfo->frags. Appending those frags to another skb's
> > frags without fixing up the page refcount can lead to UAF.
> >
> > When either the last skb in the GRO chain (the one we would append
> > frags to) or the source skb is zerocopy, skip the frags copy, and just
> > append the new skb to the frag_list.
> >
> > This is probably a bit less efficient than calling
> > skb_zcopy_downgrade_managed(), but then we'd also have to handle the
> > rest of the zerocopy flags/machinery. This can be improved in
> > net-next.
> >
> > Fixes: 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
> > Reported-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> > Assisted-by: Claude:claude-mythos-preview
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > ---
> >  net/core/gro.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > Huzaifa has found this to be exploitable to overwrite the page cache
> >
> > diff --git a/net/core/gro.c b/net/core/gro.c
> > index 31d21de5b15a..cae0a0dbfa69 100644
> > --- a/net/core/gro.c
> > +++ b/net/core/gro.c
> > @@ -123,6 +123,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
> >         lp = NAPI_GRO_CB(p)->last;
> >         pinfo = skb_shinfo(lp);
> >
> > +       if (skb_zcopy(skb) || skb_zcopy(lp))
> > +               goto merge;
> > +
> >         if (headlen <= offset) {
> >                 skb_frag_t *frag;
> >                 skb_frag_t *frag2;
> 
> Do we really want to merge these skbs in the first place?
> 
> What about play safe?

Seems reasonable to me. I'll wait another 23.5 hours to submit v2.
(and I'll let you pick some commit tags if you want)

-- 
Sabrina

Re: [PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[Pavel Begunkov]

On 5/19/26 13:40, Sabrina Dubroca wrote:
> skb_gro_receive() can currently copy frags between the source and GRO
> skb, without checking the zerocopy status, and in particular the
> SKBFL_MANAGED_FRAG_REFS flag.
> 
> When SKBFL_MANAGED_FRAG_REFS is set, the skb doesn't hold a reference
> on the pages in shinfo->frags. Appending those frags to another skb's
> frags without fixing up the page refcount can lead to UAF.
> 
> When either the last skb in the GRO chain (the one we would append
> frags to) or the source skb is zerocopy, skip the frags copy, and just
> append the new skb to the frag_list.

Was it reproduced? Sounds like we're missing skb_orphan_frags_rx()
as skbs looping into rx should be orphaned.

+cc Willem

> This is probably a bit less efficient than calling
> skb_zcopy_downgrade_managed(), but then we'd also have to handle the
> rest of the zerocopy flags/machinery. This can be improved in
> net-next.
> 
> Fixes: 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
> Reported-by: Huzaifa Sidhpurwala <huzaifas@redhat.com>
> Assisted-by: Claude:claude-mythos-preview
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>   net/core/gro.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> Huzaifa has found this to be exploitable to overwrite the page cache
> 
> diff --git a/net/core/gro.c b/net/core/gro.c
> index 31d21de5b15a..cae0a0dbfa69 100644
> --- a/net/core/gro.c
> +++ b/net/core/gro.c
> @@ -123,6 +123,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
>   	lp = NAPI_GRO_CB(p)->last;
>   	pinfo = skb_shinfo(lp);
>   
> +	if (skb_zcopy(skb) || skb_zcopy(lp))
> +		goto merge;
> +
>   	if (headlen <= offset) {
>   		skb_frag_t *frag;
>   		skb_frag_t *frag2;

-- 
Pavel Begunkov

Re: [PATCH net] net: gro: don't copy frags between mixed zcopy skbs

[Sabrina Dubroca]

2026-05-19, 15:39:19 +0100, Pavel Begunkov wrote:
> On 5/19/26 13:40, Sabrina Dubroca wrote:
> > skb_gro_receive() can currently copy frags between the source and GRO
> > skb, without checking the zerocopy status, and in particular the
> > SKBFL_MANAGED_FRAG_REFS flag.
> > 
> > When SKBFL_MANAGED_FRAG_REFS is set, the skb doesn't hold a reference
> > on the pages in shinfo->frags. Appending those frags to another skb's
> > frags without fixing up the page refcount can lead to UAF.
> > 
> > When either the last skb in the GRO chain (the one we would append
> > frags to) or the source skb is zerocopy, skip the frags copy, and just
> > append the new skb to the frag_list.
> 
> Was it reproduced? Sounds like we're missing skb_orphan_frags_rx()
> as skbs looping into rx should be orphaned.
> 
> +cc Willem

Yes, as I wrote in the patch:

Huzaifa has found this to be exploitable to overwrite the page cache

Neither of us is that familiar with MANAGED_FRAG_REFS/iouring, so
maybe there's a better way to fix this at another location in the
stack.

-- 
Sabrina