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
| ||
|
Date: Tue, 20 Mar 2018 06:11:07 +0000 From: "Yuan, Linyu (NSB - CN/Shanghai)" <linyu.yuan@...ia-sbell.com> To: Yonghong Song <yhs@...com>, "edumazet@...gle.com" <edumazet@...gle.com>, "ast@...com" <ast@...com>, "daniel@...earbox.net" <daniel@...earbox.net>, "diptanu@...com" <diptanu@...com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> CC: "kernel-team@...com" <kernel-team@...com> Subject: RE: [PATCH net-next 1/2] net: permit skb_segment on head_frag frag_list skb Sorry, I should not add "Here cause next BUG_ON always false." It cause misunderstanding, I just comment on BUG_ON in else branch. > -----Original Message----- > From: Yonghong Song [mailto:yhs@...com] > Sent: Tuesday, March 20, 2018 1:54 PM > To: Yuan, Linyu (NSB - CN/Shanghai); edumazet@...gle.com; ast@...com; > daniel@...earbox.net; diptanu@...com; netdev@...r.kernel.org > Cc: kernel-team@...com > Subject: Re: [PATCH net-next 1/2] net: permit skb_segment on head_frag > frag_list skb > > > > On 3/19/18 10:30 PM, Yuan, Linyu (NSB - CN/Shanghai) wrote: > > > > > >> -----Original Message----- > >> From: netdev-owner@...r.kernel.org > [mailto:netdev-owner@...r.kernel.org] > >> On Behalf Of Yonghong Song > >> Sent: Tuesday, March 20, 2018 1:16 PM > >> To: edumazet@...gle.com; ast@...com; daniel@...earbox.net; > >> diptanu@...com; netdev@...r.kernel.org > >> Cc: kernel-team@...com > >> Subject: [PATCH net-next 1/2] net: permit skb_segment on head_frag > frag_list > >> skb > >> > >> > >> while (pos < offset + len) { > >> if (i >= nfrags) { > >> - BUG_ON(skb_headlen(list_skb)); > >> + if (skb_headlen(list_skb) && check_list_skb == list_skb) { > > Here cause next BUG_ON always false. > > The idea is since in this branch, we did not do list_skb = > list_skb->next. So we update check_list_skb. Next time, when the > control reaches here, list_skb may still be the same, but check_list_skb > is not, so we proceed to process list_skb->frags in the else branch. > > In the else branch, we have > list_skb = list_skb->next; > check_list_skb = list_skb; > > So when the current frags are processed and ready for the list_skb. > list_skb will be equal to check_list_skb and it will be processed again. > > It is a little bit convoluted. Please let me know you have better idea. > > >> + } else { > >> + BUG_ON(skb_headlen(list_skb) && check_list_skb == > >> list_skb); > > Just according code logic, no need BUG_ON, right? > > Oh, yes, we do not need this. Will remove in the next version. > > >> > >> - i = 0; > >> - nfrags = skb_shinfo(list_skb)->nr_frags; > >> - frag = skb_shinfo(list_skb)->frags; > >> - frag_skb = list_skb; > >> + i = 0; > >> + nfrags = skb_shinfo(list_skb)->nr_frags; > >> + frag = skb_shinfo(list_skb)->frags; > >> + frag_skb = list_skb; > >> > >> - BUG_ON(!nfrags); > >> + BUG_ON(!nfrags); > >> > >> - if (skb_orphan_frags(frag_skb, GFP_ATOMIC) || > >> - skb_zerocopy_clone(nskb, frag_skb, > >> - GFP_ATOMIC)) > >> - goto err; > >> + if (skb_orphan_frags(frag_skb, GFP_ATOMIC) || > >> + skb_zerocopy_clone(nskb, frag_skb, > >> GFP_ATOMIC)) > >> + goto err; > >> > >> - list_skb = list_skb->next; > >> + list_skb = list_skb->next; > >> + check_list_skb = list_skb; > >> + } > >> } > >> > >> if (unlikely(skb_shinfo(nskb)->nr_frags >= > >> -- > >> 2.9.5 > >
Powered by blists - more mailing lists