| Message ID | 20231024235551.2769174-2-song@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: File verification with LSM and fsverity | expand |
On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: > > kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs > to access the dynptr data. They are also useful for in kernel functions > that access dynptr data, for example, bpf_verify_pkcs7_signature. > > Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel > functions can use them instead of accessing dynptr->data directly. > > Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of > dynptr->data. > > Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr > that they may return error pointers for BPF_DYNPTR_TYPE_XDP. > > Signed-off-by: Song Liu <song@kernel.org> > --- > include/linux/bpf.h | 4 ++++ > kernel/bpf/helpers.c | 16 ++++++++-------- > kernel/trace/bpf_trace.c | 15 +++++++++++---- > 3 files changed, 23 insertions(+), 12 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index b4825d3cdb29..3ed3ae37cbdf 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { > > int bpf_dynptr_check_size(u32 size); > u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > + void *buffer__opt, u32 buffer__szk); > +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > + void *buffer__opt, u32 buffer__szk); > > #ifdef CONFIG_BPF_JIT > int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index e46ac288a108..af5059f11e83 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) > * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in > * the bpf program. > * > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only > - * data slice (can be either direct pointer to the data or a pointer to the user > - * provided buffer, with its contents containing the data, if unable to obtain > - * direct pointer) > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer Hold on, nope, this one shouldn't return error pointer because it's used from BPF program side and BPF program is checking for NULL only. Does it actually return error pointer, though? I'm generally skeptical of allowing to call kfuncs directly from internal kernel code, tbh, and concerns like this are one reason why. BPF verifier sets up various conditions that kfuncs have to follow, and it seems error-prone to mix this up with internal kernel usage. > + * to a read-only data slice (can be either direct pointer to the data or a > + * pointer to the user provided buffer, with its contents containing the data, > + * if unable to obtain direct pointer) > */ > __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > void *buffer__opt, u32 buffer__szk) > @@ -2354,10 +2354,10 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset > * bpf_dynptr_slice_rdwr will not invalidate any ctx->data/data_end pointers in > * the bpf program. > * > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a > - * data slice (can be either direct pointer to the data or a pointer to the user > - * provided buffer, with its contents containing the data, if unable to obtain > - * direct pointer) > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > + * to a data slice (can be either direct pointer to the data or a pointer to the > + * user provided buffer, with its contents containing the data, if unable to > + * obtain direct pointer) > */ > __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > void *buffer__opt, u32 buffer__szk) > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index df697c74d519..2626706b6387 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1378,6 +1378,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > struct bpf_dynptr_kern *sig_ptr, > struct bpf_key *trusted_keyring) > { > + void *data, *sig; > int ret; > > if (trusted_keyring->has_ref) { > @@ -1394,10 +1395,16 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > return ret; > } > > - return verify_pkcs7_signature(data_ptr->data, > - __bpf_dynptr_size(data_ptr), > - sig_ptr->data, > - __bpf_dynptr_size(sig_ptr), > + data = bpf_dynptr_slice(data_ptr, 0, NULL, 0); > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + sig = bpf_dynptr_slice(sig_ptr, 0, NULL, 0); > + if (IS_ERR(sig)) > + return PTR_ERR(sig); > + > + return verify_pkcs7_signature(data, __bpf_dynptr_size(data_ptr), > + sig, __bpf_dynptr_size(sig_ptr), > trusted_keyring->key, > VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > NULL); > -- > 2.34.1 > >
On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: > > > > kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs > > to access the dynptr data. They are also useful for in kernel functions > > that access dynptr data, for example, bpf_verify_pkcs7_signature. > > > > Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel > > functions can use them instead of accessing dynptr->data directly. > > > > Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of > > dynptr->data. > > > > Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr > > that they may return error pointers for BPF_DYNPTR_TYPE_XDP. > > > > Signed-off-by: Song Liu <song@kernel.org> > > --- > > include/linux/bpf.h | 4 ++++ > > kernel/bpf/helpers.c | 16 ++++++++-------- > > kernel/trace/bpf_trace.c | 15 +++++++++++---- > > 3 files changed, 23 insertions(+), 12 deletions(-) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index b4825d3cdb29..3ed3ae37cbdf 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { > > > > int bpf_dynptr_check_size(u32 size); > > u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > > +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > > + void *buffer__opt, u32 buffer__szk); > > +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > > + void *buffer__opt, u32 buffer__szk); > > > > #ifdef CONFIG_BPF_JIT > > int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > > index e46ac288a108..af5059f11e83 100644 > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) > > * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in > > * the bpf program. > > * > > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only > > - * data slice (can be either direct pointer to the data or a pointer to the user > > - * provided buffer, with its contents containing the data, if unable to obtain > > - * direct pointer) > > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > > Hold on, nope, this one shouldn't return error pointer because it's > used from BPF program side and BPF program is checking for NULL only. > Does it actually return error pointer, though? So I just checked the code (should have done it before replying, sorry). It is a bug that slipped through when adding bpf_xdp_pointer() usage. We should always return NULL from this kfunc on error conditions. Let's fix it separately, but please don't change the comments. > > I'm generally skeptical of allowing to call kfuncs directly from > internal kernel code, tbh, and concerns like this are one reason why. > BPF verifier sets up various conditions that kfuncs have to follow, > and it seems error-prone to mix this up with internal kernel usage. > Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess I'm fine exposing it directly, but it still feels like it will bite us at some point later. > > + * to a read-only data slice (can be either direct pointer to the data or a > > + * pointer to the user provided buffer, with its contents containing the data, > > + * if unable to obtain direct pointer) > > */ > > __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > > void *buffer__opt, u32 buffer__szk) > > @@ -2354,10 +2354,10 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset > > * bpf_dynptr_slice_rdwr will not invalidate any ctx->data/data_end pointers in > > * the bpf program. > > * > > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a > > - * data slice (can be either direct pointer to the data or a pointer to the user > > - * provided buffer, with its contents containing the data, if unable to obtain > > - * direct pointer) > > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > > + * to a data slice (can be either direct pointer to the data or a pointer to the > > + * user provided buffer, with its contents containing the data, if unable to > > + * obtain direct pointer) > > */ > > __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > > void *buffer__opt, u32 buffer__szk) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index df697c74d519..2626706b6387 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -1378,6 +1378,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > > struct bpf_dynptr_kern *sig_ptr, > > struct bpf_key *trusted_keyring) > > { > > + void *data, *sig; > > int ret; > > > > if (trusted_keyring->has_ref) { > > @@ -1394,10 +1395,16 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > > return ret; > > } > > > > - return verify_pkcs7_signature(data_ptr->data, > > - __bpf_dynptr_size(data_ptr), > > - sig_ptr->data, > > - __bpf_dynptr_size(sig_ptr), > > + data = bpf_dynptr_slice(data_ptr, 0, NULL, 0); > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + sig = bpf_dynptr_slice(sig_ptr, 0, NULL, 0); > > + if (IS_ERR(sig)) > > + return PTR_ERR(sig); > > + > > + return verify_pkcs7_signature(data, __bpf_dynptr_size(data_ptr), > > + sig, __bpf_dynptr_size(sig_ptr), > > trusted_keyring->key, > > VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > > NULL); > > -- > > 2.34.1 > > > >
On Thu, Nov 2, 2023 at 10:09 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: > > > > > > kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs > > > to access the dynptr data. They are also useful for in kernel functions > > > that access dynptr data, for example, bpf_verify_pkcs7_signature. > > > > > > Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel > > > functions can use them instead of accessing dynptr->data directly. > > > > > > Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of > > > dynptr->data. > > > > > > Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr > > > that they may return error pointers for BPF_DYNPTR_TYPE_XDP. > > > > > > Signed-off-by: Song Liu <song@kernel.org> > > > --- > > > include/linux/bpf.h | 4 ++++ > > > kernel/bpf/helpers.c | 16 ++++++++-------- > > > kernel/trace/bpf_trace.c | 15 +++++++++++---- > > > 3 files changed, 23 insertions(+), 12 deletions(-) > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > index b4825d3cdb29..3ed3ae37cbdf 100644 > > > --- a/include/linux/bpf.h > > > +++ b/include/linux/bpf.h > > > @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { > > > > > > int bpf_dynptr_check_size(u32 size); > > > u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > > > +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > > > + void *buffer__opt, u32 buffer__szk); > > > +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > > > + void *buffer__opt, u32 buffer__szk); > > > > > > #ifdef CONFIG_BPF_JIT > > > int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > > > index e46ac288a108..af5059f11e83 100644 > > > --- a/kernel/bpf/helpers.c > > > +++ b/kernel/bpf/helpers.c > > > @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) > > > * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in > > > * the bpf program. > > > * > > > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only > > > - * data slice (can be either direct pointer to the data or a pointer to the user > > > - * provided buffer, with its contents containing the data, if unable to obtain > > > - * direct pointer) > > > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > > > > Hold on, nope, this one shouldn't return error pointer because it's > > used from BPF program side and BPF program is checking for NULL only. > > Does it actually return error pointer, though? > > So I just checked the code (should have done it before replying, > sorry). It is a bug that slipped through when adding bpf_xdp_pointer() > usage. We should always return NULL from this kfunc on error > conditions. Let's fix it separately, but please don't change the > comments. > > > > > I'm generally skeptical of allowing to call kfuncs directly from > > internal kernel code, tbh, and concerns like this are one reason why. > > BPF verifier sets up various conditions that kfuncs have to follow, > > and it seems error-prone to mix this up with internal kernel usage. > > > > Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you > want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess > I'm fine exposing it directly, but it still feels like it will bite us > at some point later. Ok, now I'm at patch #5. Think about what you are doing here. You are asking bpf_dynptr_slice_rdrw() if you can get a directly writable pointer to a data area of length *zero*. So if it's SKB, for example, then yeah, you'll be granted a pointer. But then you are proceeding to write up to sizeof(struct fsverity_digest) bytes, and that can cross into non-contiguous memory. So I'll take it back, let's not expose this kfunc directly to kernel code. Let's have a separate internal helper that will return either valid pointer or NULL for a given dynptr, but will require valid non-zero max size. Something with the signature like below void * __bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); If ptr can provide a direct pointer to memory of length *len*, great. If not, return NULL. This will be an appropriate internal API for all the use cases you are adding where we will be writing back into dynptr from other kernel APIs with the assumption of contiguous memory region. > > > > > + * to a read-only data slice (can be either direct pointer to the data or a > > > + * pointer to the user provided buffer, with its contents containing the data, > > > + * if unable to obtain direct pointer) > > > */ > > > __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > > > void *buffer__opt, u32 buffer__szk) > > > @@ -2354,10 +2354,10 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset > > > * bpf_dynptr_slice_rdwr will not invalidate any ctx->data/data_end pointers in > > > * the bpf program. > > > * > > > - * Return: NULL if the call failed (eg invalid dynptr), pointer to a > > > - * data slice (can be either direct pointer to the data or a pointer to the user > > > - * provided buffer, with its contents containing the data, if unable to obtain > > > - * direct pointer) > > > + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > > > + * to a data slice (can be either direct pointer to the data or a pointer to the > > > + * user provided buffer, with its contents containing the data, if unable to > > > + * obtain direct pointer) > > > */ > > > __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > > > void *buffer__opt, u32 buffer__szk) > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > > index df697c74d519..2626706b6387 100644 > > > --- a/kernel/trace/bpf_trace.c > > > +++ b/kernel/trace/bpf_trace.c > > > @@ -1378,6 +1378,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > > > struct bpf_dynptr_kern *sig_ptr, > > > struct bpf_key *trusted_keyring) > > > { > > > + void *data, *sig; > > > int ret; > > > > > > if (trusted_keyring->has_ref) { > > > @@ -1394,10 +1395,16 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, > > > return ret; > > > } > > > > > > - return verify_pkcs7_signature(data_ptr->data, > > > - __bpf_dynptr_size(data_ptr), > > > - sig_ptr->data, > > > - __bpf_dynptr_size(sig_ptr), > > > + data = bpf_dynptr_slice(data_ptr, 0, NULL, 0); > > > + if (IS_ERR(data)) > > > + return PTR_ERR(data); > > > + > > > + sig = bpf_dynptr_slice(sig_ptr, 0, NULL, 0); > > > + if (IS_ERR(sig)) > > > + return PTR_ERR(sig); > > > + > > > + return verify_pkcs7_signature(data, __bpf_dynptr_size(data_ptr), > > > + sig, __bpf_dynptr_size(sig_ptr), > > > trusted_keyring->key, > > > VERIFYING_UNSPECIFIED_SIGNATURE, NULL, > > > NULL); > > > -- > > > 2.34.1 > > > > > >
> On Nov 2, 2023, at 10:16 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Nov 2, 2023 at 10:09 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: >> >> On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko >> <andrii.nakryiko@gmail.com> wrote: >>> >>> On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: >>>> >>>> kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs >>>> to access the dynptr data. They are also useful for in kernel functions >>>> that access dynptr data, for example, bpf_verify_pkcs7_signature. >>>> >>>> Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel >>>> functions can use them instead of accessing dynptr->data directly. >>>> >>>> Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of >>>> dynptr->data. >>>> >>>> Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr >>>> that they may return error pointers for BPF_DYNPTR_TYPE_XDP. >>>> >>>> Signed-off-by: Song Liu <song@kernel.org> >>>> --- >>>> include/linux/bpf.h | 4 ++++ >>>> kernel/bpf/helpers.c | 16 ++++++++-------- >>>> kernel/trace/bpf_trace.c | 15 +++++++++++---- >>>> 3 files changed, 23 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >>>> index b4825d3cdb29..3ed3ae37cbdf 100644 >>>> --- a/include/linux/bpf.h >>>> +++ b/include/linux/bpf.h >>>> @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { >>>> >>>> int bpf_dynptr_check_size(u32 size); >>>> u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); >>>> +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, >>>> + void *buffer__opt, u32 buffer__szk); >>>> +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, >>>> + void *buffer__opt, u32 buffer__szk); >>>> >>>> #ifdef CONFIG_BPF_JIT >>>> int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); >>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c >>>> index e46ac288a108..af5059f11e83 100644 >>>> --- a/kernel/bpf/helpers.c >>>> +++ b/kernel/bpf/helpers.c >>>> @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) >>>> * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in >>>> * the bpf program. >>>> * >>>> - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only >>>> - * data slice (can be either direct pointer to the data or a pointer to the user >>>> - * provided buffer, with its contents containing the data, if unable to obtain >>>> - * direct pointer) >>>> + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer >>> >>> Hold on, nope, this one shouldn't return error pointer because it's >>> used from BPF program side and BPF program is checking for NULL only. >>> Does it actually return error pointer, though? Right. kfunc should not return error pointer. >> >> So I just checked the code (should have done it before replying, >> sorry). It is a bug that slipped through when adding bpf_xdp_pointer() >> usage. We should always return NULL from this kfunc on error >> conditions. Let's fix it separately, but please don't change the >> comments. >> >>> >>> I'm generally skeptical of allowing to call kfuncs directly from >>> internal kernel code, tbh, and concerns like this are one reason why. >>> BPF verifier sets up various conditions that kfuncs have to follow, >>> and it seems error-prone to mix this up with internal kernel usage. >>> >> >> Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you >> want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess >> I'm fine exposing it directly, but it still feels like it will bite us >> at some point later. > > Ok, now I'm at patch #5. Think about what you are doing here. You are > asking bpf_dynptr_slice_rdrw() if you can get a directly writable > pointer to a data area of length *zero*. So if it's SKB, for example, > then yeah, you'll be granted a pointer. But then you are proceeding to > write up to sizeof(struct fsverity_digest) bytes, and that can cross > into non-contiguous memory. > > So I'll take it back, let's not expose this kfunc directly to kernel > code. Let's have a separate internal helper that will return either > valid pointer or NULL for a given dynptr, but will require valid > non-zero max size. Something with the signature like below > > void * __bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); > > If ptr can provide a direct pointer to memory of length *len*, great. > If not, return NULL. This will be an appropriate internal API for all > the use cases you are adding where we will be writing back into dynptr > from other kernel APIs with the assumption of contiguous memory > region. Sounds good. Fixing this in the next version. Thanks, Song
On Thu, Nov 2, 2023 at 11:09 AM Song Liu <songliubraving@meta.com> wrote: > > > > > On Nov 2, 2023, at 10:16 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Nov 2, 2023 at 10:09 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > >> > >> On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko > >> <andrii.nakryiko@gmail.com> wrote: > >>> > >>> On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: > >>>> > >>>> kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs > >>>> to access the dynptr data. They are also useful for in kernel functions > >>>> that access dynptr data, for example, bpf_verify_pkcs7_signature. > >>>> > >>>> Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel > >>>> functions can use them instead of accessing dynptr->data directly. > >>>> > >>>> Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of > >>>> dynptr->data. > >>>> > >>>> Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr > >>>> that they may return error pointers for BPF_DYNPTR_TYPE_XDP. > >>>> > >>>> Signed-off-by: Song Liu <song@kernel.org> > >>>> --- > >>>> include/linux/bpf.h | 4 ++++ > >>>> kernel/bpf/helpers.c | 16 ++++++++-------- > >>>> kernel/trace/bpf_trace.c | 15 +++++++++++---- > >>>> 3 files changed, 23 insertions(+), 12 deletions(-) > >>>> > >>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h > >>>> index b4825d3cdb29..3ed3ae37cbdf 100644 > >>>> --- a/include/linux/bpf.h > >>>> +++ b/include/linux/bpf.h > >>>> @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { > >>>> > >>>> int bpf_dynptr_check_size(u32 size); > >>>> u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > >>>> +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > >>>> + void *buffer__opt, u32 buffer__szk); > >>>> +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > >>>> + void *buffer__opt, u32 buffer__szk); > >>>> > >>>> #ifdef CONFIG_BPF_JIT > >>>> int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > >>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > >>>> index e46ac288a108..af5059f11e83 100644 > >>>> --- a/kernel/bpf/helpers.c > >>>> +++ b/kernel/bpf/helpers.c > >>>> @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) > >>>> * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in > >>>> * the bpf program. > >>>> * > >>>> - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only > >>>> - * data slice (can be either direct pointer to the data or a pointer to the user > >>>> - * provided buffer, with its contents containing the data, if unable to obtain > >>>> - * direct pointer) > >>>> + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > >>> > >>> Hold on, nope, this one shouldn't return error pointer because it's > >>> used from BPF program side and BPF program is checking for NULL only. > >>> Does it actually return error pointer, though? > > Right. kfunc should not return error pointer. Turns out it doesn't, see discussion in [0]. Maybe you can sneak in that comment in your next revision as a separate lightweight patch :) [0] https://lore.kernel.org/bpf/CAEf4Bzb4VbH56S2D_5Sc3u9V=OXOy20JTr4wsObBOiUA32Md2Q@mail.gmail.com/ > > >> > >> So I just checked the code (should have done it before replying, > >> sorry). It is a bug that slipped through when adding bpf_xdp_pointer() > >> usage. We should always return NULL from this kfunc on error > >> conditions. Let's fix it separately, but please don't change the > >> comments. > >> > >>> > >>> I'm generally skeptical of allowing to call kfuncs directly from > >>> internal kernel code, tbh, and concerns like this are one reason why. > >>> BPF verifier sets up various conditions that kfuncs have to follow, > >>> and it seems error-prone to mix this up with internal kernel usage. > >>> > >> > >> Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you > >> want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess > >> I'm fine exposing it directly, but it still feels like it will bite us > >> at some point later. > > > > Ok, now I'm at patch #5. Think about what you are doing here. You are > > asking bpf_dynptr_slice_rdrw() if you can get a directly writable > > pointer to a data area of length *zero*. So if it's SKB, for example, > > then yeah, you'll be granted a pointer. But then you are proceeding to > > write up to sizeof(struct fsverity_digest) bytes, and that can cross > > into non-contiguous memory. > > > > So I'll take it back, let's not expose this kfunc directly to kernel > > code. Let's have a separate internal helper that will return either > > valid pointer or NULL for a given dynptr, but will require valid > > non-zero max size. Something with the signature like below > > > > void * __bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); > > > > If ptr can provide a direct pointer to memory of length *len*, great. > > If not, return NULL. This will be an appropriate internal API for all > > the use cases you are adding where we will be writing back into dynptr > > from other kernel APIs with the assumption of contiguous memory > > region. > > Sounds good. Fixing this in the next version. > > Thanks, > Song >
> On Nov 2, 2023, at 11:09 AM, Song Liu <songliubraving@meta.com> wrote: > > > >> On Nov 2, 2023, at 10:16 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: >> >> On Thu, Nov 2, 2023 at 10:09 AM Andrii Nakryiko >> <andrii.nakryiko@gmail.com> wrote: >>> >>> On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko >>> <andrii.nakryiko@gmail.com> wrote: >>>> >>>> On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: >>>>> >>>>> kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs >>>>> to access the dynptr data. They are also useful for in kernel functions >>>>> that access dynptr data, for example, bpf_verify_pkcs7_signature. >>>>> >>>>> Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel >>>>> functions can use them instead of accessing dynptr->data directly. >>>>> >>>>> Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of >>>>> dynptr->data. >>>>> >>>>> Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr >>>>> that they may return error pointers for BPF_DYNPTR_TYPE_XDP. >>>>> >>>>> Signed-off-by: Song Liu <song@kernel.org> >>>>> --- >>>>> include/linux/bpf.h | 4 ++++ >>>>> kernel/bpf/helpers.c | 16 ++++++++-------- >>>>> kernel/trace/bpf_trace.c | 15 +++++++++++---- >>>>> 3 files changed, 23 insertions(+), 12 deletions(-) >>>>> >>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >>>>> index b4825d3cdb29..3ed3ae37cbdf 100644 >>>>> --- a/include/linux/bpf.h >>>>> +++ b/include/linux/bpf.h >>>>> @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { >>>>> >>>>> int bpf_dynptr_check_size(u32 size); >>>>> u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); >>>>> +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, >>>>> + void *buffer__opt, u32 buffer__szk); >>>>> +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, >>>>> + void *buffer__opt, u32 buffer__szk); >>>>> >>>>> #ifdef CONFIG_BPF_JIT >>>>> int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); >>>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c >>>>> index e46ac288a108..af5059f11e83 100644 >>>>> --- a/kernel/bpf/helpers.c >>>>> +++ b/kernel/bpf/helpers.c >>>>> @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) >>>>> * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in >>>>> * the bpf program. >>>>> * >>>>> - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only >>>>> - * data slice (can be either direct pointer to the data or a pointer to the user >>>>> - * provided buffer, with its contents containing the data, if unable to obtain >>>>> - * direct pointer) >>>>> + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer >>>> >>>> Hold on, nope, this one shouldn't return error pointer because it's >>>> used from BPF program side and BPF program is checking for NULL only. >>>> Does it actually return error pointer, though? > > Right. kfunc should not return error pointer. > >>> >>> So I just checked the code (should have done it before replying, >>> sorry). It is a bug that slipped through when adding bpf_xdp_pointer() >>> usage. We should always return NULL from this kfunc on error >>> conditions. Let's fix it separately, but please don't change the >>> comments. >>> >>>> >>>> I'm generally skeptical of allowing to call kfuncs directly from >>>> internal kernel code, tbh, and concerns like this are one reason why. >>>> BPF verifier sets up various conditions that kfuncs have to follow, >>>> and it seems error-prone to mix this up with internal kernel usage. >>>> >>> >>> Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you >>> want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess >>> I'm fine exposing it directly, but it still feels like it will bite us >>> at some point later. >> >> Ok, now I'm at patch #5. Think about what you are doing here. You are >> asking bpf_dynptr_slice_rdrw() if you can get a directly writable >> pointer to a data area of length *zero*. So if it's SKB, for example, >> then yeah, you'll be granted a pointer. But then you are proceeding to >> write up to sizeof(struct fsverity_digest) bytes, and that can cross >> into non-contiguous memory. By the way, the syntax and comment of bpf_dynptr_slice() is confusing: * @buffer__opt: User-provided buffer to copy contents into. May be NULL * @buffer__szk: Size (in bytes) of the buffer if present. This is the * length of the requested slice. This must be a constant. It reads (to me) as, "if buffer__opt is NULL, buffer__szk should be 0". Is this just my confusion, or is there a real issue? Thanks, Song >> >> So I'll take it back, let's not expose this kfunc directly to kernel >> code. Let's have a separate internal helper that will return either >> valid pointer or NULL for a given dynptr, but will require valid >> non-zero max size. Something with the signature like below >> >> void * __bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); >> >> If ptr can provide a direct pointer to memory of length *len*, great. >> If not, return NULL. This will be an appropriate internal API for all >> the use cases you are adding where we will be writing back into dynptr >> from other kernel APIs with the assumption of contiguous memory >> region. >
On Thu, Nov 2, 2023 at 11:16 AM Song Liu <songliubraving@meta.com> wrote: > > > > > On Nov 2, 2023, at 11:09 AM, Song Liu <songliubraving@meta.com> wrote: > > > > > > > >> On Nov 2, 2023, at 10:16 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > >> > >> On Thu, Nov 2, 2023 at 10:09 AM Andrii Nakryiko > >> <andrii.nakryiko@gmail.com> wrote: > >>> > >>> On Thu, Nov 2, 2023 at 9:56 AM Andrii Nakryiko > >>> <andrii.nakryiko@gmail.com> wrote: > >>>> > >>>> On Tue, Oct 24, 2023 at 4:56 PM Song Liu <song@kernel.org> wrote: > >>>>> > >>>>> kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs > >>>>> to access the dynptr data. They are also useful for in kernel functions > >>>>> that access dynptr data, for example, bpf_verify_pkcs7_signature. > >>>>> > >>>>> Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel > >>>>> functions can use them instead of accessing dynptr->data directly. > >>>>> > >>>>> Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of > >>>>> dynptr->data. > >>>>> > >>>>> Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr > >>>>> that they may return error pointers for BPF_DYNPTR_TYPE_XDP. > >>>>> > >>>>> Signed-off-by: Song Liu <song@kernel.org> > >>>>> --- > >>>>> include/linux/bpf.h | 4 ++++ > >>>>> kernel/bpf/helpers.c | 16 ++++++++-------- > >>>>> kernel/trace/bpf_trace.c | 15 +++++++++++---- > >>>>> 3 files changed, 23 insertions(+), 12 deletions(-) > >>>>> > >>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h > >>>>> index b4825d3cdb29..3ed3ae37cbdf 100644 > >>>>> --- a/include/linux/bpf.h > >>>>> +++ b/include/linux/bpf.h > >>>>> @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { > >>>>> > >>>>> int bpf_dynptr_check_size(u32 size); > >>>>> u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); > >>>>> +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, > >>>>> + void *buffer__opt, u32 buffer__szk); > >>>>> +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, > >>>>> + void *buffer__opt, u32 buffer__szk); > >>>>> > >>>>> #ifdef CONFIG_BPF_JIT > >>>>> int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); > >>>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > >>>>> index e46ac288a108..af5059f11e83 100644 > >>>>> --- a/kernel/bpf/helpers.c > >>>>> +++ b/kernel/bpf/helpers.c > >>>>> @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) > >>>>> * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in > >>>>> * the bpf program. > >>>>> * > >>>>> - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only > >>>>> - * data slice (can be either direct pointer to the data or a pointer to the user > >>>>> - * provided buffer, with its contents containing the data, if unable to obtain > >>>>> - * direct pointer) > >>>>> + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer > >>>> > >>>> Hold on, nope, this one shouldn't return error pointer because it's > >>>> used from BPF program side and BPF program is checking for NULL only. > >>>> Does it actually return error pointer, though? > > > > Right. kfunc should not return error pointer. > > > >>> > >>> So I just checked the code (should have done it before replying, > >>> sorry). It is a bug that slipped through when adding bpf_xdp_pointer() > >>> usage. We should always return NULL from this kfunc on error > >>> conditions. Let's fix it separately, but please don't change the > >>> comments. > >>> > >>>> > >>>> I'm generally skeptical of allowing to call kfuncs directly from > >>>> internal kernel code, tbh, and concerns like this are one reason why. > >>>> BPF verifier sets up various conditions that kfuncs have to follow, > >>>> and it seems error-prone to mix this up with internal kernel usage. > >>>> > >>> > >>> Reading bpf_dynptr_slice_rdwr code, it does look exactly like what you > >>> want, despite the confusingly-looking 0, NULL, 0 arguments. So I guess > >>> I'm fine exposing it directly, but it still feels like it will bite us > >>> at some point later. > >> > >> Ok, now I'm at patch #5. Think about what you are doing here. You are > >> asking bpf_dynptr_slice_rdrw() if you can get a directly writable > >> pointer to a data area of length *zero*. So if it's SKB, for example, > >> then yeah, you'll be granted a pointer. But then you are proceeding to > >> write up to sizeof(struct fsverity_digest) bytes, and that can cross > >> into non-contiguous memory. > > By the way, the syntax and comment of bpf_dynptr_slice() is confusing: > > * @buffer__opt: User-provided buffer to copy contents into. May be NULL > * @buffer__szk: Size (in bytes) of the buffer if present. This is the > * length of the requested slice. This must be a constant. > > It reads (to me) as, "if buffer__opt is NULL, buffer__szk should be 0". > > Is this just my confusion, or is there a real issue? It's a bit confusing, but no, that size needs to be a "how much data do I want to read/write", so even if buffer is NULL, size has to be specified. "This is the length of the requested slice." is the most important part in that comment. > > Thanks, > Song > > > >> > >> So I'll take it back, let's not expose this kfunc directly to kernel > >> code. Let's have a separate internal helper that will return either > >> valid pointer or NULL for a given dynptr, but will require valid > >> non-zero max size. Something with the signature like below > >> > >> void * __bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len); > >> > >> If ptr can provide a direct pointer to memory of length *len*, great. > >> If not, return NULL. This will be an appropriate internal API for all > >> the use cases you are adding where we will be writing back into dynptr > >> from other kernel APIs with the assumption of contiguous memory > >> region. > > >
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index b4825d3cdb29..3ed3ae37cbdf 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1222,6 +1222,10 @@ enum bpf_dynptr_type { int bpf_dynptr_check_size(u32 size); u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr); +void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, + void *buffer__opt, u32 buffer__szk); +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, + void *buffer__opt, u32 buffer__szk); #ifdef CONFIG_BPF_JIT int bpf_trampoline_link_prog(struct bpf_tramp_link *link, struct bpf_trampoline *tr); diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index e46ac288a108..af5059f11e83 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -2270,10 +2270,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) * bpf_dynptr_slice will not invalidate any ctx->data/data_end pointers in * the bpf program. * - * Return: NULL if the call failed (eg invalid dynptr), pointer to a read-only - * data slice (can be either direct pointer to the data or a pointer to the user - * provided buffer, with its contents containing the data, if unable to obtain - * direct pointer) + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer + * to a read-only data slice (can be either direct pointer to the data or a + * pointer to the user provided buffer, with its contents containing the data, + * if unable to obtain direct pointer) */ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, void *buffer__opt, u32 buffer__szk) @@ -2354,10 +2354,10 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset * bpf_dynptr_slice_rdwr will not invalidate any ctx->data/data_end pointers in * the bpf program. * - * Return: NULL if the call failed (eg invalid dynptr), pointer to a - * data slice (can be either direct pointer to the data or a pointer to the user - * provided buffer, with its contents containing the data, if unable to obtain - * direct pointer) + * Return: NULL or error pointer if the call failed (eg invalid dynptr), pointer + * to a data slice (can be either direct pointer to the data or a pointer to the + * user provided buffer, with its contents containing the data, if unable to + * obtain direct pointer) */ __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, void *buffer__opt, u32 buffer__szk) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index df697c74d519..2626706b6387 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1378,6 +1378,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, struct bpf_dynptr_kern *sig_ptr, struct bpf_key *trusted_keyring) { + void *data, *sig; int ret; if (trusted_keyring->has_ref) { @@ -1394,10 +1395,16 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, return ret; } - return verify_pkcs7_signature(data_ptr->data, - __bpf_dynptr_size(data_ptr), - sig_ptr->data, - __bpf_dynptr_size(sig_ptr), + data = bpf_dynptr_slice(data_ptr, 0, NULL, 0); + if (IS_ERR(data)) + return PTR_ERR(data); + + sig = bpf_dynptr_slice(sig_ptr, 0, NULL, 0); + if (IS_ERR(sig)) + return PTR_ERR(sig); + + return verify_pkcs7_signature(data, __bpf_dynptr_size(data_ptr), + sig, __bpf_dynptr_size(sig_ptr), trusted_keyring->key, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
kfuncs bpf_dynptr_slice and bpf_dynptr_slice_rdwr are used by BPF programs to access the dynptr data. They are also useful for in kernel functions that access dynptr data, for example, bpf_verify_pkcs7_signature. Add bpf_dynptr_slice and bpf_dynptr_slice_rdwr to bpf.h so that kernel functions can use them instead of accessing dynptr->data directly. Update bpf_verify_pkcs7_signature to use bpf_dynptr_slice instead of dynptr->data. Also, update the comments for bpf_dynptr_slice and bpf_dynptr_slice_rdwr that they may return error pointers for BPF_DYNPTR_TYPE_XDP. Signed-off-by: Song Liu <song@kernel.org> --- include/linux/bpf.h | 4 ++++ kernel/bpf/helpers.c | 16 ++++++++-------- kernel/trace/bpf_trace.c | 15 +++++++++++---- 3 files changed, 23 insertions(+), 12 deletions(-)