Re: [RFC PATCH bpf-next v2 01/11] bpf: Set kfunc dynptr arg type flag based on prototype
From: Amery Hung <hidden>
Date: 2026-03-11 23:03:15
Also in:
bpf
On Wed, Mar 11, 2026 at 3:37 PM Andrii Nakryiko [off-list ref] wrote:
On Wed, Mar 11, 2026 at 1:01 PM Amery Hung [off-list ref] wrote:quoted
On Wed, Mar 11, 2026 at 12:44 PM Andrii Nakryiko [off-list ref] wrote:quoted
On Fri, Mar 6, 2026 at 10:44 PM Amery Hung [off-list ref] wrote:quoted
The verifier should decide whether a dynptr argument is read-only based on if the type is "const struct bpf_dynptr *", not the type of the register passed to the kfunc. This currently does not cause issues because existing kfuncs that mutate struct bpf_dynptr are constructors (e.g., bpf_dynptr_from_xxx and bpf_dynptr_clone). These kfuncs have additional check in process_dynptr_func() to make sure the stack slot does not contain initialized dynptr. Nonetheless, this should still be fixed to avoid future issues when there is a non-constructor dynptr kfunc that can mutate dynptr. This is also a small step toward unifying kfunc and helper handling in the verifier, where the first step is to generate kfunc prototype similar to bpf_func_proto before the main verification loop. We also need to correctly mark some kfunc arguments as "const struct bpf_dynptr *" to align with other kfuncs that take non-mutable dynptr argument and to not break their usage. Adding const qualifier does not break backward compatibility. Signed-off-by: Amery Hung <redacted> --- fs/verity/measure.c | 2 +- include/linux/bpf.h | 8 ++++---- kernel/bpf/helpers.c | 10 +++++----- kernel/bpf/verifier.c | 18 +++++++++++++++++- kernel/trace/bpf_trace.c | 18 +++++++++--------- tools/testing/selftests/bpf/bpf_kfuncs.h | 6 +++--- .../selftests/bpf/progs/dynptr_success.c | 6 +++--- .../bpf/progs/test_kfunc_dynptr_param.c | 7 +------ 8 files changed, 43 insertions(+), 32 deletions(-)diff --git a/fs/verity/measure.c b/fs/verity/measure.c index 6a35623ebdf0..3840436e4510 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c@@ -118,7 +118,7 @@ __bpf_kfunc_start_defs(); * * Return: 0 on success, a negative value on error. */ -__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *digest_p) +__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, const struct bpf_dynptr *digest_p)but kfunc is writing into digest_p, so that const is wrong?...quoted
{ struct bpf_dynptr_kern *digest_ptr = (struct bpf_dynptr_kern *)digest_p; const struct inode *inode = file_inode(file);[...]quoted
index 6eb6c82ed2ee..3d44896587ac 100644--- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c@@ -3000,8 +3000,8 @@ __bpf_kfunc int bpf_dynptr_clone(const struct bpf_dynptr *p, * Copies data from source dynptr to destination dynptr. * Returns 0 on success; negative error, otherwise. */ -__bpf_kfunc int bpf_dynptr_copy(struct bpf_dynptr *dst_ptr, u64 dst_off, - struct bpf_dynptr *src_ptr, u64 src_off, u64 size) +__bpf_kfunc int bpf_dynptr_copy(const struct bpf_dynptr *dst_ptr, u64 dst_off,again, dst_ptr clearly is modifiable because we are copying data into it. What am I missing, why is this logically correct? (I understand that from purely C type system POV this is fine, because we don't modify bpf_dynptr struct itself on the stack, but bpf_dynptr is a representation of some memory, and if we are modifying this memory, then I think it should be not marked as const)The patch is just to first make the arg type determination independent from bpf_reg_state and make kfunc signature consistent based on what commit 52f37c4e0f11 (bpf: Rework process_dynptr_func) has laid out. Perhaps MEM_RDONLY is a bit misleading. In process_dynptr_func(), the flag means the dynptr struct on the stack is immutable. Currently, there is no way (and maybe no need?) to specify read-only dynptr.are you basically trying to determine if CONST_DYNPTR_PTR is allowed or not?
Yes.
quoted
quoted
quoted
The verifier should decide whether a dynptr argument is read-onlycan you please remind us what "read-only dynptr argument" means for verifier?
Oh well... my apologies for the inconsistency in commit message and comments. It should be "immutable" instead of "read-only" here.
quoted
quoted
quoted
+ const struct bpf_dynptr *src_ptr, u64 src_off, u64 size) { struct bpf_dynptr_kern *dst = (struct bpf_dynptr_kern *)dst_ptr; struct bpf_dynptr_kern *src = (struct bpf_dynptr_kern *)src_ptr;[...]