Re: [PATCH RFC v2 2/4] bpf, sockmap: Restrict iter-context sockmap updates to iter/sockmap

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]

> bpf, sockmap: Restrict iter-context sockmap updates to iter/sockmap

TL;DR: Unresolved question: tracing_iter_filter() still allows
bpf_sock_destroy() from iter/sockmap, so tcp_abort() may run on a socket whose
lock isn't held. Should it be limited to iter/tcp and iter/udp?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b6c0674ff6a..4dadfe9505a2 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -10497,7 +10498,8 @@ static bool may_update_sockmap(struct bpf_verifier_env *env, int func_id)
>  	 */
>  	switch (type) {
>  	case BPF_PROG_TYPE_TRACING:
> -		if (eatype == BPF_TRACE_ITER)
> +		if (eatype == BPF_TRACE_ITER &&
> +		    !strcmp(afname + strlen(BPF_ITER_FUNC_PREFIX), "sockmap"))
>  			return true;
>  		break;
>  	case BPF_PROG_TYPE_SOCK_OPS:

Was the request to also restrict tracing_iter_filter() to iter/tcp and
iter/udp ever resolved?

In the v1 thread Alexei Starovoitov wrote:

  Pls send the PTR_TRUSTED part in v2. I'd also limit tracing_iter_filter()
  to iter/tcp and iter/udp.

The reply to that was that once bpf_iter__sockmap::sk is PTR_TRUSTED,
iter/sockmap could call bpf_sock_destroy() on an unlocked sk.  The v2 series
adds the PTR_TRUSTED marking but does not touch tracing_iter_filter(), and I
could not find a follow-up on the v2 thread:

net/core/filter.c:tracing_iter_filter() {
	if (btf_id_set8_contains(&bpf_sk_iter_kfunc_ids, kfunc_id) &&
	    prog->expected_attach_type != BPF_TRACE_ITER)
		return -EACCES;
	...
}

This still allows bpf_sock_destroy() from any BPF_TRACE_ITER program,
including iter/sockmap.

sock_map_seq_show() holds only the RCU read lock, and tcp_abort() skips
lock_sock() when called from a BPF context.  Can an iter/sockmap program
now reach tcp_abort() through bpf_sock_destroy() on a socket whose lock is
not held?

If so, should tracing_iter_filter() be limited to iter/tcp and iter/udp as
requested, or is there a reason it can stay as it is?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41373332573