On Mon, Jul 31, 2023 at 12:24 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Joanne Koong,
>
> The patch 66e3a13e7c2c: "bpf: Add bpf_dynptr_slice and
> bpf_dynptr_slice_rdwr" from Mar 1, 2023 (linux-next), leads to the
> following Smatch static checker warning:
>
> tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c:403 forward_with_gre()
> error: 'encap_gre' dereferencing possible ERR_PTR()
>
> tools/testing/selftests/bpf/progs/test_cls_redirect_dynptr.c
> 396
> 397 encap_gre = bpf_dynptr_slice_rdwr(dynptr, 0, encap_buffer, sizeof(encap_buffer));
> 398 if (!encap_gre) {
> 399 metrics->errors_total_encap_buffer_too_small++;
> 400 return TC_ACT_SHOT;
> 401 }
> 402
> --> 403 encap_gre->ip.protocol = IPPROTO_GRE;
> ^^^^^^^^^^^
>
> The bpf_dynptr_slice() function accidentally propagates error pointers
> from bpf_xdp_pointer() so it would crash here.
Good catch.
Kui-Feng, could you please send a fix?
Probably the following will be enough:
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 56ce5008aedd..eb91cae0612a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2270,7 +2270,7 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct
bpf_dynptr_kern *ptr, u32 offset
case BPF_DYNPTR_TYPE_XDP:
{
void *xdp_ptr = bpf_xdp_pointer(ptr->data, ptr->offset
+ offset, len);
- if (xdp_ptr)
+ if (!IS_ERR_OR_NULL(xdp_ptr))
return xdp_ptr;
Also I've noticed:
void bpf_xdp_copy_buf(struct xdp_buff *xdp, unsigned long off,
void *buf, unsigned long len, bool flush);
#else /* CONFIG_NET */
static inline void *bpf_xdp_pointer(struct xdp_buff *xdp, u32 offset, u32 len)
{
return NULL;
}
The latter is wrong.
Please send a separate fix.
