Re: [PATCH bpf-next 1/4] bpf: Reject exclusive maps for bpf_map_elem iterators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Generic BPF map iterators bypass BPF_F_RDONLY_PROG and map->frozen protections.
- [High] Incomplete fix for exclusive map security bypass via dynamic attach points
--

commit 9a17d32567844f5c82567e8f969e7dc780b8275c
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf: Reject exclusive maps for bpf_map_elem iterators

Exclusive maps are meant to be reachable only from the single program whose
hash matches. This patch adds a check in bpf_iter_attach_map() to reject
exclusive maps when binding to a bpf_map_elem iterator.

> diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
> index 261a03ea73d34..ae0741a09c6de 100644
> --- a/kernel/bpf/map_iter.c
> +++ b/kernel/bpf/map_iter.c
> @@ -112,6 +112,10 @@ static int bpf_iter_attach_map(struct bpf_prog *prog,
>  	map = bpf_map_get_with_uref(linfo->map.map_fd);
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
> +	if (map->excl_prog_sha) {
> +		err = -EPERM;
> +		goto put_map;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but while enforcing the
excl_prog_sha restriction here, are there other dynamic attach points that
still miss this check?

For example, looking at bpf_iter_attach_map() in net/core/bpf_sk_storage.c,
it binds a map to an sk_storage iterator without checking map->excl_prog_sha:

net/core/bpf_sk_storage.c:bpf_iter_attach_map() {
    ...
	map = bpf_map_get_with_uref(linfo->map.map_fd);
	if (IS_ERR(map))
		return PTR_ERR(map);
    ...
}

Similarly, sock_map_iter_attach_target(), sock_map_link_create(), and
sock_map_get_from_fd() in net/core/sock_map.c dynamically attach BPF programs
or iterators to sockmaps but do not appear to check map->excl_prog_sha.

Could these missing checks allow the attachment of an untrusted program or
iterator to an exclusive sk_storage or sockmap, bypassing the exclusivity
constraint?

[Severity: Critical]
This is also a pre-existing issue, but does this function properly enforce
read-only constraints on maps?

Later in this same function, the code checks if the iterator program's
max_rdwr_access exceeds the map's value_size:

kernel/bpf/map_iter.c:bpf_iter_attach_map() {
    ...
	key_acc_size = prog->aux->max_rdonly_access;
	value_acc_size = prog->aux->max_rdwr_access;
    ...
	if (key_acc_size > key_size || value_acc_size > value_size) {
		err = -EACCES;
		goto put_map;
	}
    ...
}

However, it does not appear to check if the target map has the
BPF_F_RDONLY_PROG flag or if map->frozen is set.

Since the BPF verifier defines the bpf_map_elem iterator program argument
ctx->value as PTR_TO_BUF (without MEM_RDONLY), it allows the iterator program
to write to the map element.

If an iterator is attached to a read-only or frozen map (like a .rodata array
map), can it bypass these protections and modify the contents dynamically?
Because the verifier assumes these maps are immutable and optimizes bounds
checks based on their contents, could this lead to out-of-bounds memory
accesses in previously verified BPF programs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602133052.423725-1-daniel@iogearbox.net?part=1