| Message ID | 20231102165432.1769965-2-song@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bpf: File verification with LSM and fsverity | expand |
On Thu, Nov 2, 2023 at 9:54 AM 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(-) > Oh, what a bad timing, I was too late (or too early, depending on how you look) to comment on issues I've found. Please take a look comments on v6. I think your usage of dynptr needs fixing. > 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); > -- > 2.34.1 >
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(-)