All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	eddyz87@gmail.com, kernel-team@fb.com, martin.lau@kernel.org,
	mason@kernel.org, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v3 07/20] bpf: Refuse exception cleanup shapes bpf_throw() cannot dispatch
Date: Mon, 21 Sep 2026 07:08:31 -0700	[thread overview]
Message-ID: <4342c781-b8c6-4631-ac95-7a7fdca18c94@linux.dev> (raw)
In-Reply-To: <13e6a8419b642616c50e7586ebd51734c456d19fcea3e3c6649f79b480093533@mail.kernel.org>



On 9/19/26 11:32 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index 0cd7c27a50fa..382177b80745 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -683,6 +683,7 @@ struct bpf_insn_aux_data {
>>   	bool is_iter_next; /* bpf_iter_<type>_next() kfunc call */
>>   	bool cleanup_throw_site; /* call to bpf_throw() */
>>   	bool cleanup_resume_site; /* call to bpf_unwind_resume() */
>> +	bool in_cleanup_pad; /* only runs with an exception in flight */
>>   	/*
>>   	 * 1 + the instruction index of the exception cleanup landing pad this
>>   	 * call site unwinds to, or 0 for none.
> Does the in_cleanup_pad comment accurately describe what the field
> represents at this commit? The comment says "only runs with an exception
> in flight", but cleanup_mark_pad_bodies() marks reachability from a
> landing pad without checking whether the pad is unreachable by ordinary
> control flow.
>
> For example, with a record {begin_off=0, end_off=1, landing_pad_off=1}:
>
>      0: call pcall foo        /* covered by the record */
>      1: r0 = 0                /* landing pad, also insn 0's fall-through */
>      2: call bpf_unwind_resume
>      3: exit
>
> check_cleanup_info() permits this (it only rejects a pad that lies
> *inside* a call-site range). cleanup_reachability() gives pad 1 only
> CLEANUP_REACH_RESUME, cleanup_pad_is_catch() accepts it, and insns 1-2
> get in_cleanup_pad = true. Yet on normal return from foo, execution falls
> into insns 1-2 with no exception in flight.
>
> The next commit (c046fa312ea4 "bpf: Walk the exception unwind in the
> verifier") adds the state->unwinding check that actually refuses this
> shape ("bpf_unwind_resume() at insn %d reached without an exception in
> flight"). Should the field comment say "reachable from a landing pad"
> rather than "only runs with an exception in flight" until that enforcement
> is in place, or does the check belong in this commit since the changelog
> promises to refuse every cleanup shape bpf_throw() cannot dispatch?

You are right. The comment is not good. Let us do:
   bool in_cleanup_pad; /* reachable from an exception cleanup landing pad */

>
>> diff --git a/kernel/bpf/exception.c b/kernel/bpf/exception.c
>> index cc7c8595dd64..4859d6484cc8 100644
>> --- a/kernel/bpf/exception.c
>> +++ b/kernel/bpf/exception.c
> [ ... ]
>
>> @@ -37,6 +162,255 @@ static void cleanup_mark_kfunc_sites(struct bpf_verifier_env *env)
> [ ... ]
>
>> +/* What every instruction can reach along intra-subprog edges, for
>> + * cleanup_pad_is_catch(). One backward walk over a predecessor index, rather
>> + * than a forward walk from each landing pad, which would be quadratic.
>> + */
>> +static int cleanup_reachability(struct cleanup_ctx *ctx)
>> +{
>> +	struct bpf_verifier_env *env = ctx->env;
>> +	u32 len = env->prog->len;
>> +	struct cleanup_cursor c = CLEANUP_CURSOR_INIT;
>> +	u32 *head = NULL, *link = NULL;
>> +	bool *queued = NULL;
>> +	u32 i, sp = 0;
>> +	void *scratch;
>> +	const struct cleanup_alloc_req tab[] = {
>> +		{ (void **)&head,   len,             sizeof(*head) },
>> +		{ (void **)&link,   2 * (size_t)len, sizeof(*link) },
>> +		{ (void **)&queued, len,             sizeof(*queued) },
>> +	};
>> +
>> +	scratch = cleanup_alloc(tab, ARRAY_SIZE(tab));
>> +	if (!scratch)
>> +		return -ENOMEM;
>> +
>> +	/* Index the predecessors, and seed the walk at the terminators. */
>> +	for (i = 0; i < len; i++) {
> [ ... ]
>
>> +	}
>> +
>> +	/* Each instruction re-enters the worklist at most once per bit it
>> +	 * gains, so this is linear in the number of edges.
>> +	 */
>> +	while (sp) {
> This isn't a bug, but could the two multi-line comments in
> cleanup_reachability() follow the BPF comment style with the opening /*
> on its own line? The other multi-line comments added by this series use
> that format, and the guide requires it for files under kernel/bpf/.
>
>
> ---
> 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/35492765538


  reply	other threads:[~2026-09-21 14:08 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-20  5:42 [PATCH bpf-next v3 00/20] bpf: Run exception cleanup landing pads when bpf_throw() unwinds Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 01/20] bpf: Accept the compiler's exception cleanup table at program load Yonghong Song
2026-09-20  5:56   ` sashiko-bot
2026-09-21 15:43     ` Yonghong Song
2026-09-20  6:32   ` bot+bpf-ci
2026-09-21 15:28     ` Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 02/20] bpf: Add the bpf_unwind_resume() kfunc Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 03/20] bpf: Add lookups for exception cleanup resumes and landing pads Yonghong Song
2026-09-20  6:31   ` bot+bpf-ci
2026-09-21 14:06     ` Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 04/20] bpf: Prepare for an exception cleanup table before the CFG walk Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 05/20] bpf: Make exception landing pads reachable in the CFG Yonghong Song
2026-09-20  5:42 ` [PATCH bpf-next v3 06/20] bpf: Explore the landing pads no call site reaches Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 07/20] bpf: Refuse exception cleanup shapes bpf_throw() cannot dispatch Yonghong Song
2026-09-20  6:32   ` bot+bpf-ci
2026-09-21 14:08     ` Yonghong Song [this message]
2026-09-20  5:43 ` [PATCH bpf-next v3 08/20] bpf: Walk the exception unwind in the verifier Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 09/20] bpf: Refuse a private stack for a program with an exception cleanup table Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 10/20] bpf: Dispatch exception cleanup pads from bpf_throw() Yonghong Song
2026-09-20  6:32   ` bot+bpf-ci
2026-09-21 15:55     ` Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 11/20] bpf, x86: Dispatch exception cleanup pads at run time Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 12/20] bpf, arm64: " Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 13/20] libbpf: Resolve the compiler's _Unwind_Resume to the kernel's kfunc Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 14/20] libbpf: Add cleanup_info to bpf_prog_load_opts Yonghong Song
2026-09-20  6:31   ` bot+bpf-ci
2026-09-21 15:00     ` Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 15/20] libbpf: Collect .bpf_cleanup records and pass them to the kernel Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 16/20] libbpf: Carry the exception cleanup table through the light skeleton Yonghong Song
2026-09-20  6:01   ` sashiko-bot
2026-09-21 14:14     ` Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 17/20] libbpf: Let the static linker carry .bpf_cleanup relocations Yonghong Song
2026-09-20  6:00   ` sashiko-bot
2026-09-21 14:20     ` Yonghong Song
2026-09-20  6:31   ` bot+bpf-ci
2026-09-21 14:21     ` Yonghong Song
2026-09-20  5:43 ` [PATCH bpf-next v3 18/20] selftests/bpf: Add an end-to-end .bpf_cleanup exception test Yonghong Song
2026-09-20  5:59   ` sashiko-bot
2026-09-20  5:44 ` [PATCH bpf-next v3 19/20] selftests/bpf: Cover the exception cleanup shapes the chain does not reach Yonghong Song
2026-09-20  6:01   ` sashiko-bot
2026-09-20  6:46   ` bot+bpf-ci
2026-09-21 14:27     ` Yonghong Song
2026-09-20  5:44 ` [PATCH bpf-next v3 20/20] selftests/bpf: Load an exception cleanup program from a light skeleton Yonghong Song
2026-09-20  6:31   ` bot+bpf-ci
2026-09-21 14:30     ` Yonghong Song

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=4342c781-b8c6-4631-ac95-7a7fdca18c94@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    --cc=mason@kernel.org \
    /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.