Skip to content

Conversation

@kernel-patches-daemon-bpf-rc
Copy link

Pull request for series with
subject: bpf: Introduce file dynptr
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1011975

Remove unnecessary kfunc prototypes from test programs, these are
provided by vmlinux.h

Signed-off-by: Mykyta Yatsenko <[email protected]>
Acked-by: Eduard Zingerman <[email protected]>
Dynptr currently caps size and offset at 24 bits, which isn’t sufficient
for file-backed use cases; even 32 bits can be limiting. Refactor dynptr
helpers/kfuncs to use 64-bit size and offset, ensuring consistency
across the APIs.

This change does not affect internals of xdp, skb or other dynptrs,
which continue to behave as before.

The widening enables large-file access support via dynptr, implemented
in the next patches.

Signed-off-by: Mykyta Yatsenko <[email protected]>
Move struct freader and prototypes of the functions operating on it into
the buildid.h.

This allows reusing freader outside buildid, e.g. for file dynptr
support added later.

Signed-off-by: Mykyta Yatsenko <[email protected]>
freader_fetch currently reads from at most two folios. When a read spans
into a third folio, the overflow bytes are copied adjacent to the second
folio’s data instead of being handled as a separate folio.
This patch modifies fetch algorithm to support reading from many folios.

Signed-off-by: Mykyta Yatsenko <[email protected]>
…nptr()

Move the const dynptr check into unmark_stack_slots_dynptr() so callers
don’t have to duplicate it. This puts the validation next to the code
that manipulates dynptr stack slots and allows upcoming changes to reuse
it directly.

Signed-off-by: Mykyta Yatsenko <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Acked-by: Eduard Zingerman <[email protected]>
Mark vm_area_struct in bpf_find_vma callback as trusted, also mark its
field struct file *vm_file as trusted or NULL.

Signed-off-by: Mykyta Yatsenko <[email protected]>
Add the necessary verifier plumbing for the new file-backed dynptr type.
Introduce two kfuncs for its lifecycle management:
 * bpf_dynptr_from_file() for initialization
 * bpf_dynptr_file_discard() for destruction

Currently there is no mechanism for kfunc to release dynptr, this patch
add one:
 * Dynptr release function sets meta->release_regno
 * Call unmark_stack_slots_dynptr() if meta->release_regno is set and
 dynptr ref_obj_id is set as well.

Signed-off-by: Mykyta Yatsenko <[email protected]>
Add support for file dynptr.

Introduce struct bpf_dynptr_file_impl to hold internal state for file
dynptrs, with 64-bit size and offset support.

Introduce lifecycle management kfuncs:
  - bpf_dynptr_from_file() for initialization
  - bpf_dynptr_file_discard() for destruction

Extend existing helpers to support file dynptrs in:
  - bpf_dynptr_read()
  - bpf_dynptr_slice()

Signed-off-by: Mykyta Yatsenko <[email protected]>
Move kfunc specialization (function address substitution) to later stage
of verification to support a new use case, where we need to take into
consideration whether kfunc is called in sleepable context.

Minor refactoring in add_kfunc_call(), making sure that if function
fails, kfunc desc is not added to tab->descs (previously it could be
added or not, depending on what failed).

Signed-off-by: Mykyta Yatsenko <[email protected]>
File dynptr reads may sleep when the requested folios are not in
the page cache. To avoid sleeping in non-sleepable contexts while still
supporting valid sleepable use, given that dynptrs are non-sleepable by
default, enable sleeping only when bpf_dynptr_from_file() is invoked
from a sleepable context.

This change:
  * Introduces a sleepable constructor: bpf_dynptr_from_file_sleepable()
  * Override non-sleepable constructor with sleepable if it's always
  called in sleepable context

Signed-off-by: Mykyta Yatsenko <[email protected]>
Introducing selftests for validating file-backed dynptr works as
expected.
 * validate implementation supports dynptr slice and read operations
 * validate destructors should be paired with initializers
 * validate sleepable progs can page in.

Signed-off-by: Mykyta Yatsenko <[email protected]>
@kernel-patches-daemon-bpf-rc
Copy link
Author

Upstream branch: 48a97ff
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1011975
version: 2

@kernel-patches-daemon-bpf-rc
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1011975 expired. Closing PR.

@kernel-patches-review-bot
Copy link

commit 10a07c41933a30133b36775ffa7d132a0f56ab85
Author: Mykyta Yatsenko <[email protected]>

bpf: add plumbing for file-backed dynptr

Add the necessary verifier plumbing for the new file-backed dynptr type.
Introduce two kfuncs for its lifecycle management. The patch marks
file-backed dynptrs as refcounted and integrates them into the verifier
type system.

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d353ca34c..6cfd1aa4f 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -792,12 +792,15 @@ enum bpf_type_flag {
>  	/* DYNPTR points to skb_metadata_end()-skb_metadata_len() */
>  	DYNPTR_TYPE_SKB_META	= BIT(19 + BPF_BASE_TYPE_BITS),
>
> +	/* DYNPTR points to file */
> +	DYNPTR_TYPE_FILE	= BIT(20 + BPF_BASE_TYPE_BITS),
> +
>  	__BPF_TYPE_FLAG_MAX,
>  	__BPF_TYPE_LAST_FLAG	= __BPF_TYPE_FLAG_MAX - 1,
>  };
>
>  #define DYNPTR_TYPE_FLAG_MASK	(DYNPTR_TYPE_LOCAL | DYNPTR_TYPE_RINGBUF | DYNPTR_TYPE_SKB \
> -				 | DYNPTR_TYPE_XDP | DYNPTR_TYPE_SKB_META)
> +				 | DYNPTR_TYPE_XDP | DYNPTR_TYPE_SKB_META | DYNPTR_TYPE_FILE)
>
>  /* Max number of base types. */
>  #define BPF_BASE_TYPE_LIMIT	(1UL << BPF_BASE_TYPE_BITS)
> @@ -1385,6 +1388,8 @@ enum bpf_dynptr_type {
>  	BPF_DYNPTR_TYPE_XDP,
>  	/* Points to skb_metadata_end()-skb_metadata_len() */
>  	BPF_DYNPTR_TYPE_SKB_META,
> +	/* Underlying data is a file */
> +	BPF_DYNPTR_TYPE_FILE,
>  };
>
>  int bpf_dynptr_check_size(u64 size);
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 2fd8fc249..fadd2d2ba 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -4249,6 +4249,16 @@ __bpf_kfunc int bpf_task_work_schedule_resume(struct task_struct *task, struct b
>  	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_RESUME);
>  }
>
> +__bpf_kfunc int bpf_dynptr_from_file(struct file *file, u32 flags, struct bpf_dynptr *ptr__uninit)
> +{
> +	return 0;
> +}
> +
> +__bpf_kfunc int bpf_dynptr_file_discard(struct bpf_dynptr *dynptr)
> +{
> +	return 0;
> +}
> +

The stub implementation of bpf_dynptr_from_file() returns success but
doesn't initialize the dynptr. The verifier will mark the dynptr as
initialized due to the ptr__uninit annotation, but the runtime function
leaves it uninitialized. If a BPF program successfully calls this
function and then tries to use the dynptr, what happens?

The new BPF_DYNPTR_TYPE_FILE is added to the type system, but several
runtime helpers are missing support for it. Specifically:

In __bpf_dynptr_read() (helpers.c around line 1765), the switch
statement on dynptr type includes cases for LOCAL, RINGBUF, SKB, XDP,
and SKB_META, but not FILE. Similarly missing in:
  - __bpf_dynptr_write() (helpers.c around line 1819)
  - bpf_dynptr_data() (helpers.c around line 1882)
  - bpf_dynptr_slice() (helpers.c around line 2697)

When any of these helpers are called with a FILE dynptr, they hit the
default case which calls WARN_ONCE() and returns an error. Is this the
intended behavior for the plumbing stage, or should the runtime helpers
also handle FILE dynptrs?

The concern is that BPF programs can create file-backed dynptrs (once
the stub is filled in), but attempting to read, write, or slice them
will trigger kernel warnings and fail. This seems inconsistent with
having the type fully integrated into the verifier.

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a33ab6175..7bae81c63 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -717,7 +721,7 @@ static enum bpf_type_flag get_dynptr_type_flag(enum bpf_dynptr_type type)
>
>  static bool dynptr_type_refcounted(enum bpf_dynptr_type type)
>  {
> -	return type == BPF_DYNPTR_TYPE_RINGBUF;
> +	return type == BPF_DYNPTR_TYPE_RINGBUF || type == BPF_DYNPTR_TYPE_FILE;
>  }

[ ... ]

> @@ -13323,6 +13331,11 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
>  				dynptr_arg_type |= DYNPTR_TYPE_XDP;
>  			} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_skb_meta]) {
>  				dynptr_arg_type |= DYNPTR_TYPE_SKB_META;
> +			} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_file]) {
> +				dynptr_arg_type |= DYNPTR_TYPE_FILE;
> +			} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_file_discard]) {
> +				dynptr_arg_type |= DYNPTR_TYPE_FILE;
> +				meta->release_regno = regno;
>  			} else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_clone] &&
>  				   (dynptr_arg_type & MEM_UNINIT)) {
>  				enum bpf_dynptr_type parent_type = meta->initialized_dynptr.type;

[ ... ]

> @@ -14003,7 +14016,15 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	 * PTR_TO_BTF_ID in bpf_kfunc_arg_meta, do the release now.
>  	 */
>  	if (meta.release_regno) {
> -		err = release_reference(env, regs[meta.release_regno].ref_obj_id);
> +		struct bpf_reg_state *reg = &regs[meta.release_regno];
> +
> +		if (meta.initialized_dynptr.ref_obj_id) {
> +			err = unmark_stack_slots_dynptr(env, reg);
> +			if (err)
> +				return err;
> +		} else {
> +			err = release_reference(env, reg->ref_obj_id);
> +		}
>  		if (err) {
>  			verbose(env, "kfunc %s#%d reference has not been acquired before\n",
>  				func_name, meta.func_id);

The verifier changes for the release path look correct.



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: bpf: add plumbing for file-backed dynptr
CI run summary: https://github.com/kernel-patches/bpf-rc/actions/runs/18535656109

@kernel-patches-review-bot
Copy link

commit 8ac50f23d1f98ef8a24b7c6f38412b4f7c1965fa
Author: Mykyta Yatsenko <[email protected]>

bpf: add kfuncs and helpers support for file dynptrs

This patch introduces file dynptr support in BPF by adding a new
struct bpf_dynptr_file_impl for internal state management and two
lifecycle management kfuncs: bpf_dynptr_from_file() for initialization
and bpf_dynptr_file_discard() for destruction.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index fadd2d2ba..1e63f934b 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -28,6 +28,7 @@
>  #include <linux/verification.h>
>  #include <linux/task_work.h>
>  #include <linux/irq_work.h>
> +#include <linux/buildid.h>
>
>  #include "../../lib/kstrtox.h"
>
> @@ -1653,6 +1654,13 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = {
>  	.arg2_btf_id  = BPF_PTR_POISON,
>  };
>
> +struct bpf_dynptr_file_impl {
> +	struct freader freader;
> +	/* 64 bit offset and size overriding 32 bit ones in bpf_dynptr_kern */
> +	u64 offset;
> +	u64 size;
> +};
> +

[ ... ]

> @@ -4249,13 +4304,45 @@ __bpf_kfunc int bpf_task_work_schedule_resume(struct task_struct *task, struct b
>  	return bpf_task_work_schedule(task, tw, map__map, callback, aux__prog, TWA_RESUME);
>  }
>
> -__bpf_kfunc int bpf_dynptr_from_file(struct file *file, u32 flags, struct bpf_dynptr *ptr__uninit)
> +static int make_file_dynptr(struct file *file, u32 flags, bool may_sleep,
> +			    struct bpf_dynptr_kern *ptr)
>  {
> +	struct bpf_dynptr_file_impl *state;
> +
> +	/* flags is currently unsupported */
> +	if (flags) {
> +		bpf_dynptr_set_null(ptr);
> +		return -EINVAL;
> +	}
> +
> +	state = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_dynptr_file_impl));
> +	if (!state) {
> +		bpf_dynptr_set_null(ptr);
> +		return -ENOMEM;
> +	}
> +	state->offset = 0;
> +	state->size = U64_MAX; /* Don't restrict size, as file may change anyways */
> +	freader_init_from_file(&state->freader, NULL, 0, file, may_sleep);
> +	bpf_dynptr_init(ptr, state, BPF_DYNPTR_TYPE_FILE, 0, 0);
> +	bpf_dynptr_set_rdonly(ptr);
>  	return 0;
>  }
>
> +__bpf_kfunc int bpf_dynptr_from_file(struct file *file, u32 flags, struct bpf_dynptr *ptr__uninit)
> +{
> +	return make_file_dynptr(file, flags, false, (struct bpf_dynptr_kern *)ptr__uninit);
> +}
> +

bpf_dynptr_from_file() allocates memory via bpf_mem_alloc() in
make_file_dynptr(). Looking at other resource-allocating kfuncs like
bpf_obj_new_impl(), bpf_task_acquire(), and bpf_cgroup_acquire(), they
are all marked with KF_ACQUIRE. Should bpf_dynptr_from_file() also be
marked with KF_ACQUIRE?

Without KF_ACQUIRE, the verifier won't track the acquired resource
properly, which could lead to reference leaks if the BPF program exits
without calling bpf_dynptr_file_discard().

>  __bpf_kfunc int bpf_dynptr_file_discard(struct bpf_dynptr *dynptr)
>  {
> +	struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)dynptr;
> +	struct bpf_dynptr_file_impl *df = ptr->data;
> +
> +	if (!df)
> +		return 0;
> +
> +	freader_cleanup(&df->freader);
> +	bpf_mem_free(&bpf_global_ma, df);
>  	return 0;
>  }
>

Two concerns with bpf_dynptr_file_discard():

1. Should this be marked with KF_RELEASE? Other resource-releasing
kfuncs like bpf_obj_drop_impl(), bpf_task_release(), and
bpf_cgroup_release() all use KF_RELEASE. Without it, the verifier won't
properly release the reference acquired by bpf_dynptr_from_file().

2. After freeing df with bpf_mem_free(), ptr->data still points to freed
memory. Compare this with bpf_ringbuf_discard_dynptr() which calls
bpf_dynptr_set_null() after cleanup:

  BPF_CALL_2(bpf_ringbuf_discard_dynptr, ...)
  {
      if (!ptr->data)
          return 0;
      bpf_ringbuf_commit(ptr->data, flags, true);
      bpf_dynptr_set_null(ptr);  <-- clears freed pointer
      return 0;
  }

Should bpf_dynptr_file_discard() follow the same pattern and call
bpf_dynptr_set_null() after bpf_mem_free()?

[ ... ]

> @@ -4442,9 +4532,11 @@ BTF_KFUNCS_START(common_btf_ids)
>  BTF_ID_FLAGS(func, bpf_wq_set_callback_impl, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_task_work_schedule_signal, KF_TRUSTED_ARGS)
>  BTF_ID_FLAGS(func, bpf_task_work_schedule_resume, KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_dynptr_from_file, KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_dynptr_file_discard, KF_TRUSTED_ARGS)
>  BTF_KFUNCS_END(common_btf_ids)
>

These kfunc declarations are currently only marked with KF_TRUSTED_ARGS.
Based on the resource management patterns:
- bpf_dynptr_from_file should be: KF_ACQUIRE | KF_TRUSTED_ARGS
- bpf_dynptr_file_discard should be: KF_RELEASE | KF_TRUSTED_ARGS



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: bpf: add kfuncs and helpers support for file dynptrs
CI run summary: https://github.com/kernel-patches/bpf-rc/actions/runs/18535656109

@kernel-patches-review-bot
Copy link

commit 29743ff07aa12d341a873ae556883395df25a8e8
Author: Mykyta Yatsenko <[email protected]>

lib/freader: support reading more than 2 folios

This patch extends freader_fetch to support reading from more than two
folios by introducing a loop that iterates through multiple folios
instead of handling only two.

> diff --git a/lib/buildid.c b/lib/buildid.c
> index df06e4928..ade01d7ff 100644
> --- a/lib/buildid.c
> +++ b/lib/buildid.c
> @@ -108,18 +108,20 @@ const void *freader_fetch(struct freader *r, loff_t file_off, size_t sz)
>  	 */
>  	folio_sz = folio_size(r->folio);
>  	if (file_off + sz > r->folio_off + folio_sz) {
> -		int part_sz = r->folio_off + folio_sz - file_off;
> -
> -		/* copy the part that resides in the current folio */
> -		memcpy(r->buf, r->addr + (file_off - r->folio_off), part_sz);
> -
> -		/* fetch next folio */
> -		r->err = freader_get_folio(r, r->folio_off + folio_sz);
> -		if (r->err)
> -			return NULL;
> -
> -		/* copy the rest of requested data */
> -		memcpy(r->buf + part_sz, r->addr, sz - part_sz);
> +		u64 part_sz = r->folio_off + folio_sz - file_off, off;
> +
> +		memcpy(r->buf, r->addr + file_off - r->folio_off, part_sz);
> +		off = part_sz;
> +
> +		while (off < sz) {
> +			/* fetch next folio */
> +			r->err = freader_get_folio(r, r->folio_off + folio_sz);
                                                                 ^^^^^^^^^
Can this calculate the wrong offset? The variable folio_sz is set once
before the loop begins and captures the size of the first folio. Inside
the loop, freader_get_folio() fetches new folios and updates r->folio_off
to point to each new folio's starting position. However, folio_sz is
never updated to reflect the current folio's size.

When folios have different sizes (as with multi-page folios), the
calculation 'r->folio_off + folio_sz' will use a stale folio_sz value
from iterations ago. Consider this scenario reading 15000 bytes:

  - folio_0: file offsets [0, 4096), size=4096
  - folio_1: file offsets [4096, 12288), size=8192
  - folio_2: file offsets [12288, 16384), size=4096

Loop iteration 1: freader_get_folio(0 + 4096) correctly fetches folio_1
  After: r->folio_off = 4096, folio_sz = 4096 (unchanged)

Loop iteration 2: freader_get_folio(4096 + 4096) requests offset 8192
  Expected: should request offset 12288 to fetch folio_2
  Actual: requests 8192, which is still within folio_1
  Result: freader_get_folio's cache check succeeds, reuses folio_1

The memcpy then copies from r->addr (the start of folio_1) instead of
advancing to folio_2, causing data corruption where bytes from earlier
in the file are duplicated into the buffer instead of the correct bytes.

Should folio_sz be updated after each freader_get_folio() call, perhaps
with: folio_sz = folio_size(r->folio)?

> +			if (r->err)
> +				return NULL;
> +			part_sz = min_t(u64, sz - off, folio_size(r->folio));
> +			memcpy(r->buf + off, r->addr, part_sz);
> +			off += part_sz;
> +		}
>
>  		return r->buf;
>  	}



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: lib/freader: support reading more than 2 folios
CI run summary: https://github.com/kernel-patches/bpf-rc/actions/runs/18535656109

@kernel-patches-daemon-bpf-rc kernel-patches-daemon-bpf-rc bot deleted the series/1011975=>bpf-next branch October 18, 2025 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants