Re: [PATCH bpf-next v3 1/2] bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incomplete Deadlock Fix / Bypass Vector
--

commit 11c2a6f499580e1830b9a3c86513eb4f154d7710
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf, sockmap: disallow update and delete from tc, xdp and flow_dissector

This commit aims to fix a circular locking dependency between
sk_callback_lock and stab->lock by disallowing sockmap updates and deletes
from tc, xdp, and flow_dissector programs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -8489,11 +8489,7 @@ static bool may_update_sockmap(struct bpf_verifier_env *env, int func_id)
>  			return true;
>  		break;
>  	case BPF_PROG_TYPE_SOCKET_FILTER:
> -	case BPF_PROG_TYPE_SCHED_CLS:
> -	case BPF_PROG_TYPE_SCHED_ACT:
> -	case BPF_PROG_TYPE_XDP:
>  	case BPF_PROG_TYPE_SK_REUSEPORT:
> -	case BPF_PROG_TYPE_FLOW_DISSECTOR:
>  	case BPF_PROG_TYPE_SK_LOOKUP:
>  		return true;

[Severity: High]
This is a pre-existing issue, but does leaving BPF_PROG_TYPE_SOCKET_FILTER
and BPF_PROG_TYPE_SOCK_OPS (located just above this block) allowed to
update/delete sockmaps leave the deadlock unresolved?

The commit message describes a deadlock where sk_callback_lock is held for
read, a TCP ACK goes out egress, and a BPF program modifies the sockmap,
acquiring stab->lock.

Can BPF_PROG_TYPE_SOCK_OPS execute synchronously during this same egress
path and trigger the same issue?

sk_psock_strp_data_ready()  // holds sk_callback_lock
   __tcp_cleanup_rbuf()
      tcp_send_ack()
         __tcp_transmit_skb()
            bpf_skops_write_hdr_opt()  // runs BPF_PROG_TYPE_SOCK_OPS

Similarly, can BPF_PROG_TYPE_SOCKET_FILTER be executed synchronously during
egress when dev_queue_xmit_nit() calls run_filter()?

If either of these programs calls a sockmap map update or delete helper,
wouldn't they acquire stab->lock and recreate the exact same circular
locking dependency?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629172704.1302218-1-rhkrqnwk98@gmail.com?part=1