All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Amery Hung" <ameryhung@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call
Date: Thu, 04 Jun 2026 22:28:23 +0000	[thread overview]
Message-ID: <20260604222824.2D4961F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260604220904.1032145-2-ameryhung@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error check on `acquire_reference()` in `do_check_common` allows BPF programs to bypass reference tracking and leak kernel resources.
--

commit 259ddea87c590cd0902d75313fa1c99925c708fb
Author: Amery Hung <ameryhung@gmail.com>

bpf: Fix dead error check on acquire_reference() in check_kfunc_call

This commit changes the type of the 'id' variable from a u32 to an int
so that negative error codes returned from acquire_reference() are
checked correctly instead of being treated as large positive integers.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8ed484cb1a8a4..6446db9628aed 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12817,9 +12817,10 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	struct bpf_kfunc_call_arg_meta meta;
>  	struct bpf_insn_aux_data *insn_aux;
>  	int err, insn_idx = *insn_idx_p;
> -	u32 i, nargs, ptr_type_id, id;
>  	const struct btf_param *args;
> +	u32 i, nargs, ptr_type_id;
>  	struct btf *desc_btf;
> +	int id;

[Severity: High]
This isn't a bug introduced by this patch, but since we are fixing a missing
error check on acquire_reference() here, could a similar issue exist in
do_check_common() when initializing struct_ops program arguments?

In kernel/bpf/verifier.c:do_check_common():

    /* Acquire references for struct_ops program arguments tagged with "__ref" */
    if (!subprog && env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
        for (i = 0; i < aux->ctx_arg_info_size; i++)
            aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
                              acquire_reference(env, 0, 0) : 0;
    }

Since acquire_reference() can return a negative error like -ENOMEM, assigning
it directly to aux->ctx_arg_info[i].ref_id (which is a u32) without checking
for errors seems like it would swallow the error code.

Does this allow the verifier to successfully accept a BPF program without
tracking the reference, potentially leading to a kernel resource leak when
the reference isn't released at runtime?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604220904.1032145-1-ameryhung@gmail.com?part=1

  reply	other threads:[~2026-06-04 22:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
2026-06-04 22:28   ` sashiko-bot [this message]
2026-06-04 22:43   ` bot+bpf-ci
2026-06-04 23:16   ` Eduard Zingerman
2026-06-04 22:09 ` [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR Amery Hung
2026-06-04 22:23   ` sashiko-bot
2026-06-04 22:59   ` bot+bpf-ci
2026-06-04 23:21     ` Eduard Zingerman
2026-06-04 23:27       ` Amery Hung
2026-06-05 17:57         ` Amery Hung
2026-06-05 18:04           ` Eduard Zingerman
2026-06-05 18:09             ` Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test Amery Hung
2026-06-04 22:21   ` sashiko-bot
2026-06-04 23:22   ` Eduard Zingerman
2026-06-04 22:14 ` [PATCH bpf-next v1 0/3] Object relationship refactor followup Kumar Kartikeya Dwivedi
2026-06-04 23:20   ` Amery Hung
2026-06-05 21:20 ` patchwork-bot+netdevbpf

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=20260604222824.2D4961F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ameryhung@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.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.