On 2023-08-29 10:07:59 +0200, Eric Dumazet wrote: > On Tue, Aug 29, 2023 at 8:50 AM Mohamed Khalfella > <mkhalfella@xxxxxxxxxxxxxxx> wrote: > > > > On 2023-08-28 21:18:16 -0700, willemjdebruijn wrote: > > > 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.
