> -----Original Message----- > From: Stanislav Fomichev [mailto:sdf@xxxxxxxxxx] > Sent: Wednesday, March 16, 2022 11:09 PM > To: liujian (CE) <liujian56@xxxxxxxxxx> > Cc: John Fastabend <john.fastabend@xxxxxxxxx>; Martin KaFai Lau > <kafai@xxxxxx>; ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; andrii@xxxxxxxxxx; > songliubraving@xxxxxx; yhs@xxxxxx; kpsingh@xxxxxxxxxx; > davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > bpf@xxxxxxxxxxxxxxx > Subject: Re: [PATCH bpf-next] net: Use skb->len to check the validity of the > parameters in bpf_skb_load_bytes > > On Wed, Mar 16, 2022 at 6:08 AM liujian (CE) <liujian56@xxxxxxxxxx> wrote: > > > > > > > > > -----Original Message----- > > > From: John Fastabend [mailto:john.fastabend@xxxxxxxxx] > > > Sent: Wednesday, March 16, 2022 12:00 PM > > > To: liujian (CE) <liujian56@xxxxxxxxxx>; Martin KaFai Lau > > > <kafai@xxxxxx> > > > Cc: ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; andrii@xxxxxxxxxx; > > > songliubraving@xxxxxx; yhs@xxxxxx; john.fastabend@xxxxxxxxx; > > > kpsingh@xxxxxxxxxx; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; > > > sdf@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx > > > Subject: RE: [PATCH bpf-next] net: Use skb->len to check the > > > validity of the parameters in bpf_skb_load_bytes > > > > > > liujian (CE) wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Martin KaFai Lau [mailto:kafai@xxxxxx] > > > > > Sent: Wednesday, March 16, 2022 3:58 AM > > > > > To: liujian (CE) <liujian56@xxxxxxxxxx> > > > > > Cc: ast@xxxxxxxxxx; daniel@xxxxxxxxxxxxx; andrii@xxxxxxxxxx; > > > > > songliubraving@xxxxxx; yhs@xxxxxx; john.fastabend@xxxxxxxxx; > > > > > kpsingh@xxxxxxxxxx; davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; > > > > > sdf@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx > > > > > Subject: Re: [PATCH bpf-next] net: Use skb->len to check the > > > > > validity of the parameters in bpf_skb_load_bytes > > > > > > > > > > On Tue, Mar 15, 2022 at 08:39:16PM +0800, Liu Jian wrote: > > > > > > The data length of skb frags + frag_list may be greater than > > > > > > 0xffff, so here use skb->len to check the validity of the parameters. > > > > > What is the use case that needs to look beyond 0xffff ? > > > > > > > I use sockmap with strparser, the stm->strp.offset (the begin of > > > > one application layer protocol message) maybe beyond 0xffff, but i > > > > need load the message head to do something. > > > > > > This would explain skb_load_bytes but not the other two right? Also > > > if we > > Yes, I just see that these two functions have the same judgment. > > > are doing this why not just remove those two checks in > > > flow_dissector_load() I think skb_header_pointer() does duplicate > checks. > > > Please check. > > Yes, skb_header_pointer() have checked as below, and I will send v2 to > remove 0xffff check. > > ----skb_header_pointer > > -------- __skb_header_pointer > > ------------skb_copy_bits > > ---------------- if (offset > (int)skb->len - len) > > --------------------goto fault; > > > > Thank you~ > > Do we need to have at least "offset <= 0x7fffffff" check? IOW, do we need > to enforce the unsignedness of the offset? Or does skb_header_pointer et > all properly work with the negative offsets? Yes, skb_header_pointer can not handle the negative offset. I sent a new patch. Please help review it again. Thank you. https://patchwork.kernel.org/project/netdevbpf/patch/20220317135940.358774-1-liujian56@xxxxxxxxxx/
