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?
