linux-kernel - Re: [PATCH v2 16/23] libbpf: introduce bpf_map__attach_struct_ops

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: <87o6pruf9j.fsf@linux.dev>
Date: Tue, 28 Oct 2025 10:07:04 -0700
From: Roman Gushchin <roman.gushchin@...ux.dev>
To: bot+bpf-ci@...nel.org
Cc: akpm@...ux-foundation.org,  linux-kernel@...r.kernel.org,
 ast@...nel.org, surenb@...gle.com, mhocko@...nel.org,
 shakeel.butt@...ux.dev, hannes@...xchg.org, andrii@...nel.org,
 inwardvessel@...il.com, linux-mm@...ck.org, cgroups@...r.kernel.org,
 bpf@...r.kernel.org, martin.lau@...nel.org, song@...nel.org,
 memxor@...il.com, tj@...nel.org, daniel@...earbox.net, eddyz87@...il.com,
 yonghong.song@...ux.dev, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH v2 16/23] libbpf: introduce
 bpf_map__attach_struct_ops_opts()

bot+bpf-ci@...nel.org writes:

> ```
> commit b58d54ed56054e3a550e02c324982feb20a2671e
> Author: Roman Gushchin <roman.gushchin@...ux.dev>
>
> libbpf: introduce bpf_map__attach_struct_ops_opts()
>
> This commit introduces a new API function that extends the existing
> bpf_map__attach_struct_ops() to accept additional options, specifically
> allowing attachment of struct_ops maps to cgroups via a relative_fd.
>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index b90574f39..be56a5dee 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -13196,12 +13196,19 @@ static int bpf_link__detach_struct_ops(struct bpf_link *link)
>>  	return close(link->fd);
>>  }
>>
>> -struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
>> +struct bpf_link *bpf_map__attach_struct_ops_opts(const struct bpf_map *map,
>> +						 const struct bpf_struct_ops_opts *opts)
>>  {
>> +	DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_opts);
>>  	struct bpf_link_struct_ops *link;
>>  	__u32 zero = 0;
>>  	int err, fd;
>>
>> +	if (!OPTS_VALID(opts, bpf_struct_ops_opts)) {
>> +		pr_warn("map '%s': invalid opts\n", map->name);
>> +		return libbpf_err_ptr(-EINVAL);
>> +	}
>> +
>>  	if (!bpf_map__is_struct_ops(map)) {
>>  		pr_warn("map '%s': can't attach non-struct_ops map\n", map->name);
>>  		return libbpf_err_ptr(-EINVAL);
>
> [ ... ]
>
>>  	link = calloc(1, sizeof(*link));
>>  	if (!link)
>>  		return libbpf_err_ptr(-EINVAL);
>
> Should this return -ENOMEM instead of -EINVAL? All other calloc failures
> in libbpf.c return -ENOMEM (see lines 11006, 11802, 12288, 12746, 12829,
> 12891, 13089, 13129 in the same file).

This code was introduced by e9fc3ce99b34 ("libbpf: Streamline error
reporting for high-level APIs"), so it's new. I agree that ENOMEM is
correct, however it might be too late to fix. I'll let libbpf
maintainers to decide.

>
> [ ... ]
>
>> -	fd = bpf_link_create(map->fd, 0, BPF_STRUCT_OPS, NULL);
>> +	link_opts.cgroup.relative_fd = OPTS_GET(opts, relative_fd, 0);
>> +
>> +	fd = bpf_link_create(map->fd, 0, BPF_STRUCT_OPS, &link_opts);
>
> Is the expected_revision field being propagated correctly? The new
> bpf_struct_ops_opts structure includes an expected_revision field
> (see libbpf.h), and bpf_link_create() with BPF_STRUCT_OPS reads
> opts->cgroup.expected_revision (see bpf.c line 889), but this code only
> sets relative_fd in link_opts. Should it also include:
>
>     link_opts.cgroup.expected_revision = OPTS_GET(opts,
>     expected_revision, 0);

Correct, fixed.

Thanks!