netdev - Re: [PATCH bpf-next v5 3/7] bpf: Pin associated struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMB2axN+hsZ0VQHYvw-Z31EK-7wF7SVr2nOv9=e5h=wLW_cq+w@mail.gmail.com>
Date: Tue, 4 Nov 2025 10:10:31 -0800
From: Amery Hung <ameryhung@...il.com>
To: bot+bpf-ci@...nel.org
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, alexei.starovoitov@...il.com, 
	andrii@...nel.org, daniel@...earbox.net, tj@...nel.org, martin.lau@...nel.org, 
	kernel-team@...a.com, ast@...nel.org, eddyz87@...il.com, 
	yonghong.song@...ux.dev, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v5 3/7] bpf: Pin associated struct_ops when
 registering async callback

On Tue, Nov 4, 2025 at 10:03 AM <bot+bpf-ci@...nel.org> wrote:
>
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 865b0dae3..557570479 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
>
> [ ... ]
>
> > +static int bpf_async_res_get(struct bpf_async_res *res, struct bpf_prog *prog)
> > +{
> > +     struct bpf_map *st_ops_assoc = NULL;
> > +     int err;
> > +
> > +     prog = bpf_prog_inc_not_zero(prog);
> > +     if (IS_ERR(prog))
> > +             return PTR_ERR(prog);
> > +
> > +     st_ops_assoc = READ_ONCE(prog->aux->st_ops_assoc);
> > +     if (prog->type == BPF_PROG_TYPE_STRUCT_OPS &&
> > +         st_ops_assoc && st_ops_assoc != BPF_PTR_POISON) {
> > +             st_ops_assoc = bpf_map_inc_not_zero(st_ops_assoc);
> > +             if (IS_ERR(st_ops_assoc)) {
> > +                     err = PTR_ERR(st_ops_assoc);
> > +                     goto put_prog;
> > +             }
> > +     }
>
> Can this race with bpf_prog_disassoc_struct_ops()? Between reading
> st_ops_assoc and incrementing it, another thread could dissociate the
> map:

This is fine. struct_ops map will wait for struct_ops BPF programs to
finish running (an RCU gp) before freeing itself. Therefore, when
another thread is freeing the map and this thread is calling
bpf_async_res_get(), if this thread still sees
prog->aux->st_ops_assoc, bpf_map_inc_not_zero() will fail and stop the
async callback from being registered.

>
>   bpf_async_res_get():
>     READ_ONCE(prog->aux->st_ops_assoc)  // reads valid map pointer
>
>   bpf_prog_disassoc_struct_ops():
>     guard(mutex)(&prog->aux->st_ops_assoc_mutex)
>     WRITE_ONCE(prog->aux->st_ops_assoc, NULL)
>     bpf_map_put(st_ops_assoc)  // potentially frees map
>
>   bpf_async_res_get():
>     bpf_map_inc_not_zero(st_ops_assoc)  // use-after-free
>
> The map could be freed via RCU and memory reused before
> bpf_map_inc_not_zero() executes. Other functions that access
> st_ops_assoc (bpf_prog_assoc_struct_ops and bpf_prog_disassoc_struct_ops)
> hold prog->aux->st_ops_assoc_mutex. Additionally, bpf_map_inc_not_zero's
> documentation states "map_idr_lock should have been held or the map
> should have been protected by rcu read lock."
>
> Should bpf_async_res_get() hold the st_ops_assoc_mutex or an RCU read
> lock around the st_ops_assoc access?
>
> > +
> > +     res->prog = prog;
> > +     res->st_ops_assoc = st_ops_assoc;
> > +     return 0;
> > +put_prog:
> > +     bpf_prog_put(prog);
> > +     return err;
> > +}
>
> [ ... ]
>
>
> ---
> 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/19077679684