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: Thu, 10 Feb 2022 21:54:23 -0800 From: Maciej Żenczykowski <maze@...gle.com> To: Lina Wang <lina.wang@...iatek.com> Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Linux NetDev <netdev@...r.kernel.org>, Kernel hackers <linux-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Willem Bruijn <willemb@...gle.com>, Eric Dumazet <edumazet@...gle.com>, zhuoliang.zhang@...iatek.com, chao.song@...iatek.com Subject: Re: [PATCH] net: fix wrong network header length On Thu, Feb 10, 2022 at 8:12 PM Lina Wang <lina.wang@...iatek.com> wrote: > > On Thu, 2022-02-10 at 17:02 +0100, Paolo Abeni wrote: > > > > @@ -3682,6 +3682,7 @@ struct sk_buff *skb_segment_list(struct > > > sk_buff *skb, > > > struct sk_buff *tail = NULL; > > > struct sk_buff *nskb, *tmp; > > > int err; > > > + unsigned int len_diff = 0; > > > > Mintor nit: please respect the reverse x-mas tree order. > > > > Yes,v2 has change unsigned int to int Reverse christmas tree, means from longest to shortest, like so: struct sk_buff *tail = NULL; struct sk_buff *nskb, *tmp; + int len_diff = 0; int err; That said, I think the = 0 is not needed, so this can be just + int len_diff, err; > > > > > > > skb_push(skb, -skb_network_offset(skb) + offset); > > > @@ -3721,9 +3722,11 @@ struct sk_buff *skb_segment_list(struct > > > sk_buff *skb, > > > skb_push(nskb, -skb_network_offset(nskb) + offset); > > > > > > skb_release_head_state(nskb); > > > + len_diff = skb_network_header_len(nskb) - > > > skb_network_header_len(skb); > > > __copy_skb_header(nskb, skb); > > > > > > skb_headers_offset_update(nskb, skb_headroom(nskb) - > > > skb_headroom(skb)); > > > + nskb->transport_header += len_diff; > > > > This does not look correct ?!? the network hdr position for nskb will > > still be uncorrect?!? and even the mac hdr likely?!? possibly you > > need > > to change the offset in skb_headers_offset_update(). > > > > Network hdr position and mac hdr are both right, because bpf processing & > skb_headers_offset_update have updated them to right position. After bpf > loading, the first skb's network header&mac_header became 44, transport > header still is 64. After skb_headers_offset_update, fraglist skb's mac > header and network header are still 24, the same with original packet. > Just fraglist skb's transport header became 44, as original is 64. > Only transport header cannot be easily updated the same offset, because > 6to4 has different network header. > > Actually,at the beginning, I want to change skb_headers_offset_update, but > it has been called also in other place, maybe a new function should be > needed here. > > Skb_headers_offset_update has other wrong part in my scenary, > inner_transport_header\inner_network_header\inner_mac_header shouldnot be > changed, but they are been updated because of different headroom. They are > not used later, so wrong value didnot affect anything. > > > Paolo > > > > Thanks!Maciej Żenczykowski, Kernel Networking Developer @ Google
Powered by blists - more mailing lists