On 2023-08-29 10:07:59 +0200, Eric Dumazet wrote:

On Tue, Aug 29, 2023 at 8:50 AM Mohamed Khalfella
[off-list ref] wrote:

quoted

On 2023-08-28 21:18:16 -0700, willemjdebruijn wrote:

quoted

Small point: nfrags is not the only state that needs to be refreshed
after a fags realloc, also frag.

I am new to this code. Can you help me understand why frag needs to be
updated too? My reading of this code is that frag points to frags array
in shared info. As long as shared info pointer remain the same frag
pointer should remain valid.

skb_copy_ubufs() could actually call skb_unclone() and thus skb->head
could be re-allocated.

I guess that if you run your patch (and a repro of the bug ?) with
KASAN enabled kernel, you should see a possible use-after-free ?

To force the skb_unclone() path, having a tcpdump catching all packets
would be enough I think.

Okay, I see it now. I have not tested this patch with tcpdump capturing
packets at the same time. Also, during my testing I have not seen the
value of skb->head changnig. Now you are mentioning it it, I will make
sure to test with tcpdump running and see skb->head changing. Thank you
for pointing that out.

For frag, I guess something like frag = &skb_shinfo(list_skb)->frags[i];
should do the job. I have not tested it though. I will need to do more
testing before posting updated patch.