All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program
  2026-10-17 16:37 [PATCH bpf-next v2 00/43] bpf: Memory safety, lifetime and locking fixes Alexei Starovoitov
@ 2026-10-17 16:37 ` Alexei Starovoitov
  2026-10-17 17:17   ` bot+bpf-ci
  0 siblings, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-17 16:37 UTC (permalink / raw)
  To: bpf; +Cc: daniel, andrii, eddyz87, memxor, netdev, josef, mason

From: Josef Bacik <josef@toxicpanda.com>

An extension inherits the expected_attach_type of the program it extends.
If that is BPF_TRACE_ITER, check_attach_btf_id() hands the extension to
bpf_iter_prog_supported(), which goes by the name of the attach function.
For an extension that is the name of the subprogram it replaces, so the
load fails with -EINVAL, unless that subprogram is called like the function
of a target, bpf_iter_bpf_map_elem for example.

Then the extension gets the context of that target.  If the subprogram
has as many arguments, it can use the key and the value as buffers, but
how far it reaches is noted in the extension, where the test of the
iterator link does not look: it reads and writes behind the elements of
the map.  And the id of the subprogram in the BTF of the program is cached
as the id of the target, so that programs of that target which are loaded
already cannot be attached.

Refuse all extensions here.

Fixes: 15d83c4d7cef ("bpf: Allow loading of a bpf_iter program")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/bpf_iter.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 4668c1696a08..5fd376bd210b 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -351,6 +351,13 @@ int bpf_iter_prog_supported(struct bpf_prog *prog)
 	const char *prefix = BPF_ITER_FUNC_PREFIX;
 	int prefix_len = strlen(prefix);
 
+	/*
+	 * The attach function of an extension is a subprogram of the iterator
+	 * program that it extends, whatever its name is.
+	 */
+	if (prog->type == BPF_PROG_TYPE_EXT)
+		return -EINVAL;
+
 	if (strncmp(attach_fname, prefix, prefix_len))
 		return -EINVAL;
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program
  2026-10-17 16:37 ` [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program Alexei Starovoitov
@ 2026-10-17 17:17   ` bot+bpf-ci
  0 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-10-17 17:17 UTC (permalink / raw)
  To: alexei.starovoitov, bpf
  Cc: daniel, andrii, eddyz87, memxor, netdev, josef, mason, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, mason,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 3461 bytes --]

> bpf: Don't take an extension for an iterator program

TL;DR: The new EXT check in bpf_iter_prog_supported() may be incomplete: a
TRACING/BPF_TRACE_ITER program attached to a JITed program's function can
still reach it, possibly corrupting the iterator target's btf_id cache and
ctx_arg_info.

> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index 4668c1696a08..5fd376bd210b 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -351,6 +351,13 @@ int bpf_iter_prog_supported(struct bpf_prog *prog)
>  	const char *prefix = BPF_ITER_FUNC_PREFIX;
>  	int prefix_len = strlen(prefix);
>  
> +	/*
> +	 * The attach function of an extension is a subprogram of the iterator
> +	 * program that it extends, whatever its name is.
> +	 */
> +	if (prog->type == BPF_PROG_TYPE_EXT)
> +		return -EINVAL;
> +
>  	if (strncmp(attach_fname, prefix, prefix_len))
>  		return -EINVAL;

Is this check enough, or can the same problem be reached without an
extension?

A BPF_PROG_TYPE_TRACING program loaded with expected_attach_type set to
BPF_TRACE_ITER, attach_prog_fd pointing at a JITed program P1, and
attach_btf_id set to a global function of P1 named bpf_iter_bpf_map_elem
looks like it gets here too.  bpf_prog_load_check_attach() has no
BPF_PROG_TYPE_TRACING case that rejects it, and in
bpf_check_attach_target() the BPF_TRACE_ITER case with tgt_prog set only
does:

    btf_type_is_func() / func_proto checks
    btf_distill_func_proto()

Unlike the BPF_TRACE_RAW_TP case just above it, it never rejects tgt_prog.

check_attach_btf_id() then stores the subprogram name of P1 in
prog->aux->attach_func_name and calls bpf_iter_prog_supported().  Since
prog->type is BPF_PROG_TYPE_TRACING here, the new check does not refuse it:

    bpf_check()
      check_attach_btf_id()
        bpf_check_attach_target()
        bpf_iter_prog_supported()
          cache_btf_id()
            tinfo->btf_id = prog->aux->attach_btf_id

The 'bpf_iter_' prefix matches and strcmp() against reg_info->target
matches, so cache_btf_id() caches an id from P1's BTF as the btf_id of the
target, and bpf_prog_ctx_arg_info_init() copies the target's ctx_arg_info.
This happens at check_attach_btf_id() time, so it persists even if the load
fails later.

After that, bpf_iter_link_attach() compares iter->btf_id with the poisoned
id, so already loaded programs of that target fail with -ENOENT.  A vmlinux
BTF program whose attach_btf_id equals the forged id would be matched to the
wrong target (and the wrong ctx_arg_info) by the first branch of the loop in
bpf_iter_prog_supported().

The out-of-bounds buffer access described in the commit message does not
seem to apply to this route, since the program being attached is the one
whose max_rdonly_access is recorded.  Only the btf_id cache corruption and
the ctx_arg_info mix-up remain.  It needs CAP_BPF and CAP_PERFMON, the same
as the extension case.

Would it be better to refuse any program that has a dst_prog here, for
example:

    if (prog->aux->dst_prog)
        return -EINVAL;

That also covers all extensions, since they always have one.  Alternatively,
could bpf_check_attach_target() reject tgt_prog for BPF_TRACE_ITER, as it
does for BPF_TRACE_RAW_TP?


---
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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/39051545464

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program
@ 2026-10-17 22:06 Tyrone Wu
  2026-10-18  0:39 ` Alexei Starovoitov
  0 siblings, 1 reply; 4+ messages in thread
From: Tyrone Wu @ 2026-10-17 22:06 UTC (permalink / raw)
  To: alexei.starovoitov
  Cc: andrii, bpf, daniel, eddyz87, josef, mason, memxor, netdev

Hi, I originally reported this issue a few weeks ago
(srry for no text wrapping):
https://lore.kernel.org/bpf/AyAj3bTqZ30lo9rs1fqBKO0Vk7CJqGfxcmHqHhmmEBpxlQxf0M4lfKOmSE_MbTVLhhxeFUx1PbyX6oZKq8Jh54h0pzjEFtXlRSqi4m6DcQA=@pm.me/

with the BPF global function name issue when freplace'ing a global func in
an ITER prog. I'm hoping for EXT to still support ITER progs for my
usecase, so rejecting all extensions seems extreme.

From what I understand, `tinfo` should be a registered iterator target
(`tgt_prog`/`prog->aux->dst_prog` in our case for an EXT prog),
and this is used to set `prog->aux->ctx_arg_info` for later verification.

Could `bpf_iter_prog_supported()` instead have the `tinfo` search loop be
applied on the `prog->aux->dst_prog`/`tgt_prog`, and
`bpf_prog_ctx_arg_info_init` on the original `prog`?

Ideally I'd still like EXT to support ITER prog types if possible.

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program
  2026-10-17 22:06 [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program Tyrone Wu
@ 2026-10-18  0:39 ` Alexei Starovoitov
  0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-10-18  0:39 UTC (permalink / raw)
  To: Tyrone Wu; +Cc: andrii, bpf, daniel, eddyz87, josef, mason, memxor, netdev

On Sat, Oct 17, 2026 at 03:06 PM Tyrone Wu <tkwu@pm.me> wrote:
> Hi, I originally reported this issue a few weeks ago

Sorry, I missed that report. Both issues in there are real.

> I'm hoping for EXT to still support ITER progs for my
> usecase, so rejecting all extensions seems extreme.

It never worked though. As you found the load fails unless the func
is named bpf_iter_<target>, and then it's taken for the function of
that target. The patch only closes that hole.

> Could `bpf_iter_prog_supported()` instead have the `tinfo` search loop be
> applied on the `prog->aux->dst_prog`/`tgt_prog`, and
> `bpf_prog_ctx_arg_info_init` on the original `prog`?

That's a start, but not enough.
bpf_iter_get_func_proto() goes by attach_btf_id as well.
The iter link checks max_rdonly_access and max_rdwr_access of the iter prog
against the map. The extension has its own and nobody looks at them.

At the end it's a feature, not a fix.
Send a patch and please explain the use case.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-18  0:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-17 22:06 [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program Tyrone Wu
2026-10-18  0:39 ` Alexei Starovoitov
  -- strict thread matches above, loose matches on Subject: below --
2026-10-17 16:37 [PATCH bpf-next v2 00/43] bpf: Memory safety, lifetime and locking fixes Alexei Starovoitov
2026-10-17 16:37 ` [PATCH bpf-next v2 26/43] bpf: Don't take an extension for an iterator program Alexei Starovoitov
2026-10-17 17:17   ` bot+bpf-ci

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.