On Thu, May 21, 2026 at 10:35 AM Jakub Kicinski [off-list ref] wrote:

On Thu, 21 May 2026 06:15:17 -0400 Jamal Hadi Salim wrote:

quoted

quoted

This is the same claim as sashiko1 but sashiko2 gave a much more
convincing description ;->
skb_has_shared_frag() is only true if the frags are flagged as
SKBFL_SHARED_FRAG (which is what the repro did); however, if we get
frags from eg a driver on ingress and that skb gets cloned with frags
we won't catch it.
One approach is to do an if (skb_has_any_shared_frags(skb)) and then
do a skb_linearize_cow() but that sounds like overkill.

Yeah, this would be overkill - imagine running tcpdump 100% will be cloned

quoted

Another which will make the patch even uglier (but less expensive) is
to add an extra check insde the patch's "if (write_offset < 0)"
to do:  if (write_offset + (int)sizeof(hdata) > 0) { skb_ensure_writable()}

To be precise, something like attached (untested, uncompiled)

Can we not pull the headers? Do you know of anyone modifying payloads
with pedit?

not sure - but it is not unreasonable if someone used it for such a case.

The concept of "shared frags" is silly IMHO, if I'm checking right only
rxrpc and xfrm think that it's a thing. I'm hoping to delete that and
reclaim the flag id in net-next...

All these issues stem from the shared frags point, but the patchlet i
showed is unrelated to SKBFL_SHARED_FRAG, it is more related to
cloned+frags.
If you remove it in net-next - does that necessitate removing all
"fixes" that tried to address that issue? It seems like that would be
sensible, but it makes me wonder why the shared frags concept exists
in the first place.

cheers,
jamal