netdev - Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with 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] [day] [month] [year] [list]

Message-ID: <CAMB2axPrnaEf981+drX-MPBZOrPhZvFxuq0d+2Ue-sTqvEQZ8w@mail.gmail.com>
Date: Mon, 17 Nov 2025 13:53:35 -0800
From: Amery Hung <ameryhung@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: 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, 
	bpf@...r.kernel.org, bot+bpf-ci@...nel.org
Subject: Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops

On Mon, Nov 17, 2025 at 1:39 PM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
>
>
> On 11/14/25 2:51 PM, bot+bpf-ci@...nel.org wrote:
> >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> >> index a41e6730e..626aa9197 100644
> >> --- a/kernel/bpf/bpf_struct_ops.c
> >> +++ b/kernel/bpf/bpf_struct_ops.c
> >
> > [ ... ]
> >
> >> @@ -811,6 +822,12 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
> >>              bpf_link_init(&link->link, BPF_LINK_TYPE_STRUCT_OPS,
> >>                            &bpf_struct_ops_link_lops, prog, prog->expected_attach_type);
> >>              *plink++ = &link->link;
> >> +
> >> +            err = bpf_prog_assoc_struct_ops(prog, &st_map->map);
> >> +            if (err) {
> >> +                    bpf_prog_put(prog);
> >> +                    goto reset_unlock;
> >> +            }
> >
> > Can this double-put the prog reference? In bpf_struct_ops_map_update_elem:
> >
> >    prog = bpf_prog_get(prog_fd);              // refcnt = 1
> >    bpf_link_init(..., prog, ...);             // link takes ownership
> >    *plink++ = &link->link;                    // link stored in st_map
> >    err = bpf_prog_assoc_struct_ops(prog, ...);
> >    if (err) {
> >      bpf_prog_put(prog);                      // refcnt = 0, prog freed
> >      goto reset_unlock;
> >    }
> >
> > Then at reset_unlock:
> >
> >    bpf_struct_ops_map_put_progs(st_map);      // calls bpf_link_put(link)
> >
> > The bpf_link_put will eventually call bpf_link_dealloc()->bpf_prog_put(link->prog),
> > attempting to put the already-freed prog reference. This looks like a use-after-free
> > if bpf_prog_assoc_struct_ops fails (which can happen when a non-struct_ops program
> > is already associated and returns -EBUSY).
>
> The ai-review should be correct in general on the double bpf_prog_put.
>
> >
> > Should the error path skip the bpf_prog_put and let bpf_struct_ops_map_put_progs
> > handle the cleanup via the link?
>
> bpf_prog_assoc_struct_ops will never return error for
> BPF_PROG_TYPE_STRUCT_OPS. If that is the case, maybe completely remove
> the err check.

Thanks for reviewing

Will remove the check and add comments about why the error can be ignored.

>