lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6689541517901_12869e29412@willemb.c.googlers.com.notmuch>
Date: Sat, 06 Jul 2024 10:26:29 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Fred Li <dracodingfly@...il.com>, 
 pabeni@...hat.com
Cc: aleksander.lobakin@...el.com, 
 andrii@...nel.org, 
 ast@...nel.org, 
 bpf@...r.kernel.org, 
 daniel@...earbox.net, 
 davem@...emloft.net, 
 dracodingfly@...il.com, 
 edumazet@...gle.com, 
 haoluo@...gle.com, 
 hawk@...nel.org, 
 john.fastabend@...il.com, 
 jolsa@...nel.org, 
 kpsingh@...nel.org, 
 kuba@...nel.org, 
 linux-kernel@...r.kernel.org, 
 linux@...ssschuh.net, 
 martin.lau@...ux.dev, 
 mkhalfella@...estorage.com, 
 nbd@....name, 
 netdev@...r.kernel.org, 
 sashal@...nel.org, 
 sdf@...gle.com, 
 song@...nel.org, 
 yonghong.song@...ux.dev, 
 herbert@...dor.apana.org.au
Subject: Re: [PATCH v2 1/2] net: Fix skb_segment when splitting gso_size
 mangled skb having linear-headed frag_list whose head_frag=true

Fred Li wrote:
> > I must admit I more than a bit lost in the many turns of skb_segment(),
> > but the above does not look like the correct solution, as it will make
> > the later BUG_ON() unreachable/meaningless.
> 
> Sorry, the subsequent BUG_ON maybe should be removed in this patch, because
> for skb_headlen(list_skb) > len, it will continue splitting as commit 13acc94eff122 
> (net: permit skb_segment on head_frag frag_list skb) does.
> 
> > 
> > Do I read correctly that when the BUG_ON() triggers:
> > 
> > list_skb->len is 125
> > len is 75
> > list_skb->frag_head is true
> >
> 
> yes, it's correct.
> list_skb->len is 125
> gso_size is 75, also the len in the BUG_ON conditon
> list_skb->head_frag is true
>  
> > It looks like skb_segment() is becoming even and ever more complex to
> > cope with unexpected skb layouts, only possibly when skb_segment() is
> > called by some BPF helpers.
> > 
> > Thanks,
> > 
> > Paolo
> 
> I'll wait for more suggestions from others.

In general, agreed with Paolo. Segmentation is getting ever more
complex and the code hard to follow.

Maybe at some point we'll have to bite the bullet and seriously
refactor it. Or at least parts, such as the frag_list handling case.
But that kills any odds of backporting fixes, so not if we can avoid
it.

In particular, changing gso_size on skbs with frag_list is fragile.

Instead of adding another special case, how about just linearizing
sbks after BPF calls adjust_room (at least if called without
BPF_F_ADJ_ROOM_FIXED_GSO) if they have frag_list.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ