netdev - Re: [PATCH v2 bpf-next 2/4] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMB2axOe3wMFNyn_TiFrw8iRPjW5RbmkpQBXh2fPUrVF-rkHFA@mail.gmail.com>
Date: Thu, 16 Oct 2025 16:58:04 -0700
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, 
	bpf@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH v2 bpf-next 2/4] bpf: Support associating BPF program with struct_ops

On Thu, Oct 16, 2025 at 4:51 PM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
>
> On 10/16/25 1:45 PM, Amery Hung wrote:
> > diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> > index a41e6730edcf..e060d9823e4a 100644
> > --- a/kernel/bpf/bpf_struct_ops.c
> > +++ b/kernel/bpf/bpf_struct_ops.c
> > @@ -528,6 +528,7 @@ static void bpf_struct_ops_map_put_progs(struct bpf_struct_ops_map *st_map)
> >       for (i = 0; i < st_map->funcs_cnt; i++) {
> >               if (!st_map->links[i])
> >                       break;
> > +             bpf_prog_disassoc_struct_ops(st_map->links[i]->prog);
>
> It took me some time to understand why it needs to specifically call
> bpf_prog_disassoc_struct_ops here for struct_ops programs. bpf_prog_put
> has not been done yet. The BPF_PTR_POISON could be set back to NULL. My
> understanding is the BPF_PTR_POISON should stay with the prog's lifetime?

You are right, once BPF_PTR_POISON is set, it cannot be cleared. Will
fix it in v3

> >               bpf_link_put(st_map->links[i]);
> >               st_map->links[i] = NULL;
> >       }
> > @@ -801,6 +802,9 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
> >                       goto reset_unlock;
> >               }
> >
> > +             /* If the program is reused, prog->aux->st_ops_assoc will be poisoned */
> > +             bpf_prog_assoc_struct_ops(prog, &st_map->map);
> > +
> >               link = kzalloc(sizeof(*link), GFP_USER);
> >               if (!link) {
> >                       bpf_prog_put(prog);
> > @@ -1394,6 +1398,46 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
> >       return err;
> >   }
> >
> > +int bpf_prog_assoc_struct_ops(struct bpf_prog *prog, struct bpf_map *map)
> > +{
> > +     struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
> > +     void *kdata = &st_map->kvalue.data;
> > +     int ret = 0;
> > +
> > +     mutex_lock(&prog->aux->st_ops_assoc_mutex);
> > +
> > +     if (prog->aux->st_ops_assoc && prog->aux->st_ops_assoc != kdata) {
> > +             if (prog->type == BPF_PROG_TYPE_STRUCT_OPS)
> > +                     WRITE_ONCE(prog->aux->st_ops_assoc, BPF_PTR_POISON);
> > +
> > +             ret = -EBUSY;
> > +             goto out;
> > +     }
> > +
> > +     WRITE_ONCE(prog->aux->st_ops_assoc, kdata);
> > +out:
> > +     mutex_unlock(&prog->aux->st_ops_assoc_mutex);
> > +     return ret;
> > +}
> > +
> > +void bpf_prog_disassoc_struct_ops(struct bpf_prog *prog)
> > +{
> > +     mutex_lock(&prog->aux->st_ops_assoc_mutex);
>
> Can it check the prog type here and decide if bpf_struct_ops_put needs
> to be called?

I will move map refcount inc and dec into these two helpers.

Thanks for the suggestion

>
> > +     WRITE_ONCE(prog->aux->st_ops_assoc, NULL);
>
>