All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: bot+bpf-ci@kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	kafai@fb.com, eddyz87@gmail.com, songliubraving@fb.com,
	yhs@fb.com, menglong8.dong@gmail.com, rostedt@kernel.org,
	martin.lau@kernel.org, yonghong.song@linux.dev, clm@meta.com,
	ihor.solodrai@linux.dev
Subject: Re: [PATCHv6 bpf-next 10/29] bpf: Add multi tracing attach types
Date: Thu, 28 May 2026 15:13:39 +0200	[thread overview]
Message-ID: <ahg_gxSxmDNhoGoA@krava> (raw)
In-Reply-To: <23ab5817f619274a252b35119e760817c383e3c20f9ce9facccbf4b4fec9dd58@mail.kernel.org>

On Wed, May 27, 2026 at 12:59:43PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 435ae6efe961..27ad0c265c95 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/overflow.h>
> >  #include <linux/cookie.h>
> >  #include <linux/verification.h>
> > +#include <linux/btf_ids.h>
> >
> >  #include <net/netfilter/nf_bpf_link.h>
> >  #include <net/netkit.h>
> 
> [ ... ]
> 
> > @@ -2904,6 +2914,11 @@ static int bpf_prog_mark_insn_arrays_ready(struct bpf_prog *prog)
> >  	return 0;
> >  }
> >
> > +extern int bpf_multi_func(void);
> > +int __init __used bpf_multi_func(void) { return 0; }
> 
> The signature change from v5 to v6 creates a potential issue. In v5,
> bpf_multi_func had 6 arguments (matching the initial verifier setup):
> 
>   int __init __used bpf_multi_func(u64 r1, u64 r2, u64 r3,
>                                    u64 r4, u64 r5, u64 r6)
> 
> Now it has no arguments. The Sashiko AI review in v5 identified that
> context array bounds checking could fail when bpf_multi_func's argument
> count doesn't match the attached function's argument count, and the
> author acknowledged the need to disable direct context access.
> 
> Since the initial verification happens against bpf_multi_func (which now
> has 0 arguments), but the verifier previously allowed direct ctx[0-5]
> accesses based on the 6-argument version, is there still a path where
> direct context accesses could be allowed? If so, would accessing ctx[0]
> on a function with 0 arguments lead to out-of-bounds kernel stack reads?
> 
> The earlier review suggested either:
>   (a) explicitly disabling direct memory accesses to the ctx array for
>       FENTRY_MULTI and FEXIT_MULTI, or
>   (b) enforcing the use of bounds-checked helpers like bpf_get_func_arg()
> 
> Was this implemented somewhere in the patch series?

it's implemented through bpf_multi_func without any arguments,
then verifier won't allow any access to any argument,
including ctx[0], which is 1st argument

jirka

> 
> > +
> > +BTF_ID_LIST_GLOBAL_SINGLE(bpf_multi_func_btf_id, func, bpf_multi_func)
> 
> [ ... ]
> 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index c8d980fdd709..58450cb88beb 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> 
> [ ... ]
> 
> > @@ -18837,6 +18839,11 @@ static int check_attach_modify_return(unsigned long addr, const char *func_name)
> >
> >  #endif /* CONFIG_FUNCTION_ERROR_INJECTION */
> >
> > +static bool is_tracing_multi_id(const struct bpf_prog *prog, u32 btf_id)
> > +{
> > +	return is_tracing_multi(prog->expected_attach_type) && bpf_multi_func_btf_id[0] == btf_id;
> > +}
> > +
> >  int bpf_check_attach_target(struct bpf_verifier_log *log,
> >  			    const struct bpf_prog *prog,
> >  			    const struct bpf_prog *tgt_prog,
> 
> [ ... ]
> 
> > @@ -19094,7 +19105,18 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> >  		if (ret < 0)
> >  			return ret;
> >
> > -		if (tgt_prog) {
> > +		/*
> > +		 * *.multi programs don't need an address during program
> > +		 * verification, we just take the module ref if needed.
> > +		 */
> > +		if (is_tracing_multi_id(prog, btf_id)) {
> > +			if (btf_is_module(btf)) {
> > +				mod = btf_try_get_module(btf);
> > +				if (!mod)
> > +					return -ENOENT;
> > +			}
> > +			addr = 0;
> > +		} else if (tgt_prog) {
> >  			if (subprog == 0)
> >  				addr = (long) tgt_prog->bpf_func;
> >  			else
> 
> [ ... ]
> 
> > @@ -19122,6 +19144,12 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> >  			ret = -EINVAL;
> >  			switch (prog->type) {
> >  			case BPF_PROG_TYPE_TRACING:
> > +				/* *.multi sleepable programs will pass initial sleepable check,
> > +				 * the actual attached btf ids are checked later during the link
> > +				 * attachment.
> > +				 */
> > +				if (is_tracing_multi_id(prog, btf_id))
> > +					ret = 0;
> >  				if (!check_attach_sleepable(btf_id, addr, tname))
> >  					ret = 0;
> 
> The multi types are not included in btf_validate_prog_ctx_type() in
> kernel/bpf/btf.c. That function validates which attach types allow u64*
> as their ctx parameter:
> 
> kernel/bpf/btf.c:btf_validate_prog_ctx_type() {
>     case BPF_TRACE_FENTRY:
>     case BPF_TRACE_FEXIT:
>     case BPF_MODIFY_RETURN:
>         ...
> }
> 
> Since BPF_TRACE_FENTRY_MULTI and BPF_TRACE_FEXIT_MULTI also use u64*
> context, do they need to be added to that validation list? Or if direct
> context access should be disabled for multi types (per the earlier
> review concern), should they be handled with special rejection logic?
> 
> 
> ---
> 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/26509800686


  reply	other threads:[~2026-05-28 13:13 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 11:39 [PATCHv6 bpf-next 00/29] bpf: tracing_multi link Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 01/29] ftrace: Add ftrace_hash_count function Jiri Olsa
2026-05-27 11:57   ` sashiko-bot
2026-05-28 13:10     ` Jiri Olsa
2026-05-27 12:37   ` bot+bpf-ci
2026-05-27 11:39 ` [PATCHv6 bpf-next 02/29] ftrace: Add ftrace_hash_remove function Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 03/29] ftrace: Add add_ftrace_hash_entry function Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 04/29] bpf: Use mutex lock pool for bpf trampolines Jiri Olsa
2026-05-27 12:21   ` sashiko-bot
2026-05-28 13:13     ` Jiri Olsa
2026-05-27 12:59   ` bot+bpf-ci
2026-05-27 11:39 ` [PATCHv6 bpf-next 05/29] bpf: Add struct bpf_trampoline_ops object Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 06/29] bpf: Move trampoline image setup into bpf_trampoline_ops callbacks Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 07/29] bpf: Add bpf_trampoline_add/remove_prog functions Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 08/29] bpf: Add struct bpf_tramp_node object Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 09/29] bpf: Factor fsession link to use struct bpf_tramp_node Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 10/29] bpf: Add multi tracing attach types Jiri Olsa
2026-05-27 12:59   ` bot+bpf-ci
2026-05-28 13:13     ` Jiri Olsa [this message]
2026-05-27 11:39 ` [PATCHv6 bpf-next 11/29] bpf: Move sleepable verification code to btf_id_allow_sleepable Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 12/29] bpf: Add bpf_trampoline_multi_attach/detach functions Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 13/29] bpf: Add support for tracing multi link Jiri Olsa
2026-05-27 14:34   ` sashiko-bot
2026-05-28 13:13     ` Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 14/29] bpf: Add support for tracing_multi link cookies Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 15/29] bpf: Add support for tracing_multi link session Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 16/29] bpf: Add support for tracing_multi link fdinfo Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 17/29] libbpf: Add bpf_object_cleanup_btf function Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 18/29] libbpf: Add bpf_link_create support for tracing_multi link Jiri Olsa
2026-05-27 12:09   ` sashiko-bot
2026-05-28 13:10     ` Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 19/29] libbpf: Add btf_type_is_traceable_func function Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 20/29] libbpf: Add support to create tracing multi link Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 21/29] selftests/bpf: Add tracing multi skel/pattern/ids attach tests Jiri Olsa
2026-05-27 12:01   ` sashiko-bot
2026-05-28 13:10     ` Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 22/29] selftests/bpf: Add tracing multi skel/pattern/ids module " Jiri Olsa
2026-05-27 12:59   ` bot+bpf-ci
2026-05-28 13:10     ` Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 23/29] selftests/bpf: Add tracing multi intersect tests Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 24/29] selftests/bpf: Add tracing multi cookies test Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 25/29] selftests/bpf: Add tracing multi session test Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 26/29] selftests/bpf: Add tracing multi attach fails test Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 27/29] selftests/bpf: Add tracing multi verifier " Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 28/29] selftests/bpf: Add tracing multi attach benchmark test Jiri Olsa
2026-05-27 11:39 ` [PATCHv6 bpf-next 29/29] selftests/bpf: Add tracing multi attach rollback tests Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ahg_gxSxmDNhoGoA@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kafai@fb.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=menglong8.dong@gmail.com \
    --cc=rostedt@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.