[PATCH] bpf: Fix signedness bug

[PATCH] bpf: Fix signedness bug

[Ethan Tidmore]

The function acquire_reference() returns negative error codes and 'id'
is an unsigned integer, so the check (id < 0) is always impossible.

Detected by Smatch:
kernel/bpf/verifier.c:13115 check_kfunc_call()
warn: unsigned 'id' is never less than zero.

Fixes: 308c7a0ae8859 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 kernel/bpf/verifier.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8ed484cb1a8a..aa8f10fce071 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13111,9 +13111,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		}
 		mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
 		if (is_kfunc_acquire(&meta)) {
-			id = acquire_reference(env, insn_idx, 0);
-			if (id < 0)
-				return id;
+			err = acquire_reference(env, insn_idx, 0);
+			if (err < 0)
+				return err;
+			id = err;
+
 			regs[BPF_REG_0].id = id;
 		} else if (is_rbtree_node_type(ptr_type) || is_list_node_type(ptr_type)) {
 			ref_set_non_owning(env, &regs[BPF_REG_0]);
-- 
Thanks,
ET
https://github.com/sponsors/ethantidmore

Re: [PATCH] bpf: Fix signedness bug

[bot+bpf-ci]

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

> commit cc27f12c12ff54b8eace339c3e130317bf205277
> Author:     Ethan Tidmore <ethantidmore06@gmail.com>
>
>     bpf: Fix signedness bug
>
>     The function acquire_reference() returns negative error codes and 'id'
>     is an unsigned integer, so the check (id < 0) is always impossible.

Is this commit message accurate? Looking at check_kfunc_call() in
kernel/bpf/verifier.c, the variable 'id' is declared as a signed int:

    const struct btf_param *args;
    u32 i, nargs, ptr_type_id;
    struct btf *desc_btf;
    int id;

Since acquire_reference() can return negative error codes like -ENOMEM,
wouldn't the original check (id < 0) work correctly?

The three other call sites of acquire_reference() in the verifier all use
the same signed-int pattern without issue:

    verifier.c:mark_stack_slots_dynptr():
        id = acquire_reference(env, insn_idx, dynptr_id);
        if (id < 0)
            return id;

    verifier.c:mark_stack_slots_iter():
        id = acquire_reference(env, insn_idx, ITER_TYPE_TO_ID(iter_type));
        if (id < 0)
            return id;

    verifier.c:check_helper_call():
        id = acquire_reference(env, insn_idx, fn->arg_type[arg_num]);
        if (id < 0)
            return id;

It looks like the Smatch warning might be a false positive, and the original
code was already handling the error properly.

>
>     Detected by Smatch:
>     kernel/bpf/verifier.c:13115 check_kfunc_call()
>     warn: unsigned 'id' is never less than zero.
>
>     Fixes: 308c7a0ae8859 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
>     Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 935595138aa0..be528a0be13c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13167,9 +13167,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  		}
>  		mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
>  		if (is_kfunc_acquire(&meta)) {
> -			id = acquire_reference(env, insn_idx, 0);
> -			if (id < 0)
> -				return id;
> +			err = acquire_reference(env, insn_idx, 0);
> +			if (err < 0)
> +				return err;
> +			id = err;
> +
>  			regs[BPF_REG_0].id = id;
>  		} else if (is_rbtree_node_type(ptr_type) || is_list_node_type(ptr_type)) {
>  			ref_set_non_owning(env, &regs[BPF_REG_0]);

The code change itself is behavior-equivalent since 'err' is also a signed
int, so the patch doesn't introduce a runtime regression. But if the premise
that 'id' is unsigned is wrong, should the commit message be corrected?


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

Re: [PATCH] bpf: Fix signedness bug

[Amery Hung]

On Fri, Jun 5, 2026 at 8:25 PM <bot+bpf-ci@kernel.org> wrote:
>
> > commit cc27f12c12ff54b8eace339c3e130317bf205277
> > Author:     Ethan Tidmore <ethantidmore06@gmail.com>
> >
> >     bpf: Fix signedness bug
> >
> >     The function acquire_reference() returns negative error codes and 'id'
> >     is an unsigned integer, so the check (id < 0) is always impossible.
>
> Is this commit message accurate? Looking at check_kfunc_call() in
> kernel/bpf/verifier.c, the variable 'id' is declared as a signed int:
>
>     const struct btf_param *args;
>     u32 i, nargs, ptr_type_id;
>     struct btf *desc_btf;
>     int id;

Hi,

As pointed out by the bot, the fix to the issue has been merged as
part of the following patchset.
https://lore.kernel.org/bpf/20260605202056.1780352-1-ameryhung@gmail.com/

Thanks,
Amery

>
> Since acquire_reference() can return negative error codes like -ENOMEM,
> wouldn't the original check (id < 0) work correctly?
>
> The three other call sites of acquire_reference() in the verifier all use
> the same signed-int pattern without issue:
>
>     verifier.c:mark_stack_slots_dynptr():
>         id = acquire_reference(env, insn_idx, dynptr_id);
>         if (id < 0)
>             return id;
>
>     verifier.c:mark_stack_slots_iter():
>         id = acquire_reference(env, insn_idx, ITER_TYPE_TO_ID(iter_type));
>         if (id < 0)
>             return id;
>
>     verifier.c:check_helper_call():
>         id = acquire_reference(env, insn_idx, fn->arg_type[arg_num]);
>         if (id < 0)
>             return id;
>
> It looks like the Smatch warning might be a false positive, and the original
> code was already handling the error properly.
>
> >
> >     Detected by Smatch:
> >     kernel/bpf/verifier.c:13115 check_kfunc_call()
> >     warn: unsigned 'id' is never less than zero.
> >
> >     Fixes: 308c7a0ae8859 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
> >     Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 935595138aa0..be528a0be13c 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -13167,9 +13167,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> >               }
> >               mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
> >               if (is_kfunc_acquire(&meta)) {
> > -                     id = acquire_reference(env, insn_idx, 0);
> > -                     if (id < 0)
> > -                             return id;
> > +                     err = acquire_reference(env, insn_idx, 0);
> > +                     if (err < 0)
> > +                             return err;
> > +                     id = err;
> > +
> >                       regs[BPF_REG_0].id = id;
> >               } else if (is_rbtree_node_type(ptr_type) || is_list_node_type(ptr_type)) {
> >                       ref_set_non_owning(env, &regs[BPF_REG_0]);
>
> The code change itself is behavior-equivalent since 'err' is also a signed
> int, so the patch doesn't introduce a runtime regression. But if the premise
> that 'id' is unsigned is wrong, should the commit message be corrected?
>
>
> ---
> 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/27050796130

Re: [PATCH] bpf: Fix signedness bug

[Ethan Tidmore]

On Fri Jun 5, 2026 at 10:25 PM CDT, bot+bpf-ci wrote:
>> commit cc27f12c12ff54b8eace339c3e130317bf205277
>> Author:     Ethan Tidmore <ethantidmore06@gmail.com>
>>
>>     bpf: Fix signedness bug
>>
>>     The function acquire_reference() returns negative error codes and 'id'
>>     is an unsigned integer, so the check (id < 0) is always impossible.
>
> Is this commit message accurate? Looking at check_kfunc_call() in
> kernel/bpf/verifier.c, the variable 'id' is declared as a signed int:
>
>     const struct btf_param *args;
>     u32 i, nargs, ptr_type_id;
>     struct btf *desc_btf;
>     int id;

No, 'id' is assigned as a u32:

	u32 i, nargs, ptr_type_id, id;

--
Thanks,
ET
https://github.com/sponsors/ethantidmore

Re: [PATCH] bpf: Fix signedness bug

[Amery Hung]

On Fri, Jun 5, 2026 at 8:31 PM Ethan Tidmore <ethantidmore06@gmail.com> wrote:
>
> On Fri Jun 5, 2026 at 10:25 PM CDT, bot+bpf-ci wrote:
> >> commit cc27f12c12ff54b8eace339c3e130317bf205277
> >> Author:     Ethan Tidmore <ethantidmore06@gmail.com>
> >>
> >>     bpf: Fix signedness bug
> >>
> >>     The function acquire_reference() returns negative error codes and 'id'
> >>     is an unsigned integer, so the check (id < 0) is always impossible.
> >
> > Is this commit message accurate? Looking at check_kfunc_call() in
> > kernel/bpf/verifier.c, the variable 'id' is declared as a signed int:
> >
> >     const struct btf_param *args;
> >     u32 i, nargs, ptr_type_id;
> >     struct btf *desc_btf;
> >     int id;
>
> No, 'id' is assigned as a u32:

You might want to pull bpf-next and check again then.

>
>         u32 i, nargs, ptr_type_id, id;
>
> --
> Thanks,
> ET
> https://github.com/sponsors/ethantidmore