| Message ID | 20220722220105.2065466-1-joannelkoong@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bbd52178e249fe893ef4a9b87cde5b6c473b0a7c |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v1,1/1] bpf: Fix bpf_xdp_pointer return pointer | expand |
On Fri, Jul 22, 2022 at 03:01:05PM -0700, Joanne Koong wrote: > For the case where offset + len == size, bpf_xdp_pointer should return a > valid pointer to the addr because that access is permitted. We should > only return NULL in the case where offset + len exceeds size. > > Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > net/core/filter.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 289614887ed5..4307a75eeb4c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -3918,7 +3918,7 @@ static void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len) > offset -= frag_size; > } > out: > - return offset + len < size ? addr + offset : NULL; > + return offset + len <= size ? addr + offset : NULL; This fix should be for the bpf tree. Acked-by: Martin KaFai Lau <kafai@fb.com>
> For the case where offset + len == size, bpf_xdp_pointer should return a > valid pointer to the addr because that access is permitted. We should > only return NULL in the case where offset + len exceeds size. > > Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > net/core/filter.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 289614887ed5..4307a75eeb4c 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -3918,7 +3918,7 @@ static void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len) > offset -= frag_size; > } > out: > - return offset + len < size ? addr + offset : NULL; > + return offset + len <= size ? addr + offset : NULL; > } > > BPF_CALL_4(bpf_xdp_load_bytes, struct xdp_buff *, xdp, u32, offset, > -- > 2.30.2 > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
On Mon, Jul 25, 2022 at 1:17 PM Martin KaFai Lau <kafai@fb.com> wrote: > > On Fri, Jul 22, 2022 at 03:01:05PM -0700, Joanne Koong wrote: > > For the case where offset + len == size, bpf_xdp_pointer should return a > > valid pointer to the addr because that access is permitted. We should > > only return NULL in the case where offset + len exceeds size. > > > > Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > > --- > > net/core/filter.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/core/filter.c b/net/core/filter.c > > index 289614887ed5..4307a75eeb4c 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -3918,7 +3918,7 @@ static void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len) > > offset -= frag_size; > > } > > out: > > - return offset + len < size ? addr + offset : NULL; > > + return offset + len <= size ? addr + offset : NULL; > This fix should be for the bpf tree. Ah I see. To confirm my understanding, fixes should always go to the bpf tree (unless it's fixing a patch that only resides in the bpf-next tree), correct? > > Acked-by: Martin KaFai Lau <kafai@fb.com>
On 7/25/22 11:55 PM, Joanne Koong wrote: > On Mon, Jul 25, 2022 at 1:17 PM Martin KaFai Lau <kafai@fb.com> wrote: >> >> On Fri, Jul 22, 2022 at 03:01:05PM -0700, Joanne Koong wrote: >>> For the case where offset + len == size, bpf_xdp_pointer should return a >>> valid pointer to the addr because that access is permitted. We should >>> only return NULL in the case where offset + len exceeds size. >>> >>> Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") >>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com> >>> --- >>> net/core/filter.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/net/core/filter.c b/net/core/filter.c >>> index 289614887ed5..4307a75eeb4c 100644 >>> --- a/net/core/filter.c >>> +++ b/net/core/filter.c >>> @@ -3918,7 +3918,7 @@ static void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len) >>> offset -= frag_size; >>> } >>> out: >>> - return offset + len < size ? addr + offset : NULL; >>> + return offset + len <= size ? addr + offset : NULL; >> This fix should be for the bpf tree. > Ah I see. To confirm my understanding, fixes should always go to the > bpf tree (unless it's fixing a patch that only resides in the bpf-next > tree), correct? Yes, correct. Given we're really late in the cycle with rc8 my preference is to only queue really urgent fixes to bpf tree at this point. This one I just took to bpf-next given merge window is opening this Sun, thus this will go to Linus' tree with just few days offset anyway. Thanks, Daniel
Hello: This patch was applied to bpf/bpf-next.git (master) by Daniel Borkmann <daniel@iogearbox.net>: On Fri, 22 Jul 2022 15:01:05 -0700 you wrote: > For the case where offset + len == size, bpf_xdp_pointer should return a > valid pointer to the addr because that access is permitted. We should > only return NULL in the case where offset + len exceeds size. > > Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > > [...] Here is the summary with links: - [bpf-next,v1,1/1] bpf: Fix bpf_xdp_pointer return pointer https://git.kernel.org/bpf/bpf-next/c/bbd52178e249 You are awesome, thank you!
diff --git a/net/core/filter.c b/net/core/filter.c index 289614887ed5..4307a75eeb4c 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3918,7 +3918,7 @@ static void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len) offset -= frag_size; } out: - return offset + len < size ? addr + offset : NULL; + return offset + len <= size ? addr + offset : NULL; } BPF_CALL_4(bpf_xdp_load_bytes, struct xdp_buff *, xdp, u32, offset,
For the case where offset + len == size, bpf_xdp_pointer should return a valid pointer to the addr because that access is permitted. We should only return NULL in the case where offset + len exceeds size. Fixes: 3f364222d032 ("net: xdp: introduce bpf_xdp_pointer utility routine") Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- net/core/filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)