Re: [PATCH net-next v3 2/2] net: prevent mixing frags from different memory providers or bindings

[Mina Almasry]

On Fri, Nov 6, 2026 at 2:22 PM Stanislav Fomichev <sdf.kernel@xxxxxxxxx> wrote:
>
> On 11/06, Mina Almasry wrote:
> > On Thu, Nov 5, 2026 at 8:59 AM Stanislav Fomichev <sdf.kernel@xxxxxxxxx> wrote:
> > > > +static inline void *skb_net_iov_provider(const struct sk_buff *skb)
> > > > +{
> > > > +     struct net_iov *niov;
> > > > +
> > >
> > > [..]
> > >
> > > > +     if (!skb_has_net_iovs(skb) || !skb_shinfo(skb)->nr_frags)
> > > > +             return NULL;
> > >
> > > (a few nits/questions)
> > >
> > > Why !skb_has_net_iovs is not enough? (and if !nr_frags is required,
> > > maybe fold it into skb_has_net_iovs?)
> > >
> >
> > Well skb_has_net_iovs only checks skb->unreadable, which is not
> > actually enough. The skb doesn't necessarily have net_iovs if it's
> > unreadable, it technically could be that it has no frags, right?
> >
> > Technically we should never have the edge case where
> > skb->unreadable==true && skb_shinfo->nr_frags==0, but I don't think
> > the code is careful about that today, and I don't know that I want to
> > do a global refactor everywhere we remove frags, which skb-> flags
> > need to be updated.
> >
> > but you're right that it can be folded.
>
> Not sure I'm buying that, but happy to look again if you disagree. I think
> the only way to make an skb unreadable right now is to add at least
> one niov frag? Don't think this is (ever?) gonna change? What does
> and unreadable skb without frags even means?

I have no doubt that we set skb->unreadable correctly when we add an
unreadable frag. I have doubt that we unset it correctly when we
remove the last unreadable frag (or we do it correctly for other skb
flags like pfmemalloc). If you can trust it, my LLM confirmed my
suspicion:

```
  1. skb->pfmemalloc is NEVER cleared (or recomputed) when nr_frags
becomes 0 on an sk_buff—and isn't even propagated in skb_split() /
skb_shift():
      • None of ___pskb_trim(), __pskb_pull_tail(), skb_split(),
skb_shift(), skb_condense(), gro_pull_from_frag0(), or
__pskb_trim_head() touch skb->pfmemalloc.
      • Why it's fundamentally broken to just clear skb->pfmemalloc =
0 on nr_frags == 0: Unlike struct xdp_buff (which has a separate
XDP_FLAGS_FRAGS_PF_MEMALLOC flag that BPF helpers clear via
      xdp_buff_clear_frag_pfmemalloc() in filter.c:4441-4444 and
filter.c:12740-12743 when !sinfo->nr_frags), struct sk_buff has a
single sticky skb->pfmemalloc:1 bit shared by both linear skb->head
      (skbuff.c:515-885) and frags[] (skbuff.h:2631-2632). Clearing
skb->pfmemalloc when nr_frags == 0 would wrongly clear pfmemalloc when
skb->head itself was allocated from PFMEMALLOC reserves.
  2. skb->unreadable is cleared in some nr_frags -> 0 paths, but still
missed in a few others:
      • Unlike pfmemalloc, skb->unreadable only applies to frags[] /
frag_list (linear skb->head is always CPU-readable).
      • Where we DO clear skb->unreadable when nr_frags == 0:
          • ___pskb_trim() (skbuff.c:2810-2811): if
(!skb_shinfo(skb)->nr_frags && !skb_has_frag_list(skb))
skb->unreadable = 0;
          • skb_split_inside_header(), skb_split_no_header(), and
skb_shift() (skbuff.c:4228-4454): cleared by Patch 1/2 in this series.
      • Where nr_frags can drop to 0 and skb->unreadable is NOT cleared:
          • pskb_carve_inside_nonlinear() (skbuff.c:7139-7167): if
pskb_extract() carves past all frags[] (k == 0, e.g., into frag_list),
shinfo->nr_frags = 0 on the clone without clearing skb->unreadable (if
          !skb_has_frag_list(skb) or if the remaining frag_list is readable).
          • __pskb_trim_head() (tcp_output.c:2020): sets
shinfo->nr_frags = k without checking !k (in practice tcp_trim_head()
is only called when len < skb->len and skb_headlen(skb) == 0, so k > 0
unless a
          frag_list is present).
          • skb_try_coalesce() (skbuff.c:6365) and skb_gro_receive()
(gro.c:131): zero from_shinfo->nr_frags = 0 when stealing frags
without clearing from->unreadable (benign because from is immediately
freed
          via stolen-head free).
          • __pskb_pull_tail() (skbuff.c:2894) and
gro_pull_from_frag0() (gro.c:416): cannot be entered with
skb->unreadable == 1 (__pskb_pull_tail() bails on
!skb_frags_readable(skb), and
          skb_gro_reset_offset() requires skb_frag_page(frag0) != NULL).
```

I already have 1 ambitious refactor on list to try to remove
pp_recycle. I think maybe I want to do the nr_frags checks in this
patch rather than do a global cleanup of setting skb flags correctly
so that skb->unread is 100% accurate. WDTY?

-- 
Thanks,
Mina