Re: [PATCH net 1/1] bpf: reject raw iteration over managed sk_storage values

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: bpf_link_update bypasses iterator map->record safety check
--

commit 756d826567f6b13fb5453e6e861ce172e85fb4c1
Author: Zhiling Zou <zhilinz@nebusec.ai>

bpf: reject raw iteration over managed sk_storage values

The sk_storage iterator exposes values as raw memory. This commit attaches
validation checks against value_size to prevent BPF programs from modifying
BTF-managed fields like kptrs.

> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 1d295a8769fad..b54fd90d4bde9 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -863,6 +863,12 @@ static int bpf_iter_attach_map(struct bpf_prog *prog,
>  	if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
>  		goto put_map;
>  
> +	/* The iterator exposes values as raw buffers, bypassing field checks. */
> +	if (map->record && prog->aux->max_rdwr_access) {
> +		err = -EACCES;
> +		goto put_map;
> +	}

[Severity: High]
This security check is only performed during initial attachment. Can this be
bypassed using link replacement?

If an initial BPF iterator program is loaded with max_rdwr_access == 0,
attached to an sk_storage map containing managed fields, and then replaced via
bpf_link_update with a program where max_rdwr_access > 0, won't it bypass
this check?

Looking at bpf_iter_link_replace() in kernel/bpf/bpf_iter.c:

static int bpf_iter_link_replace(struct bpf_link *link,
				 struct bpf_prog *new_prog,
				 struct bpf_prog *old_prog)
{
	...
	if (link->prog->type != new_prog->type ||
	    link->prog->expected_attach_type != new_prog->expected_attach_type ||
	    link->prog->aux->attach_btf_id != new_prog->aux->attach_btf_id) {
		ret = -EINVAL;
		goto out_unlock;
	}

	old_prog = xchg(&link->prog, new_prog);
	...
}

It swaps the program without checking the new program's max_rdwr_access
against the attached map's BTF record.

> +
>  	if (prog->aux->max_rdwr_access > map->value_size) {
>  		err = -EACCES;
>  		goto put_map;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791980149.git.zhilinz@nebusec.ai?part=1