Re: [PATCH bpf-next v6 2/6] bpf: Support associating BPF program with struct_ops

[Martin KaFai Lau <martin.lau@linux.dev>]

On 11/14/25 2:51 PM, bot+bpf-ci@kernel.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.