Re: [PATCH bpf v3 1/2] bpf, sockmap: fix use-after-free when the stream parser resizes the skb

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The patch attempts to fix a slab-use-after-free by rejecting stream parser programs that modify the SKB, relying on `prog->aux->changes_pkt_data`. However, this flag does not account for `bpf_dynptr_write`, which also modifies the SKB and triggers the exact same use-after-free vulnerability.
--

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

bpf, sockmap: fix use-after-free when the stream parser resizes the skb

This patch rejects stream parser programs that modify packet data to
prevent a use-after-free. The issue occurs when skb_ensure_writable() frees
the frag_list that the stream parser continues to track.

> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index 99e3789492a09..c60ba6d292f9a 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c

[ ... ]

> @@ -1515,6 +1515,17 @@ static int sock_map_prog_link_lookup(struct bpf_map *map, struct bpf_prog ***ppr
>  	return 0;
>  }
>  
> +static int sock_map_prog_attach_check(enum bpf_attach_type attach_type,
> +				      struct bpf_prog *prog)
> +{
> +	/* A stream parser must not modify the skb, only measure it. */
> +	if (prog && attach_type == BPF_SK_SKB_STREAM_PARSER &&
> +	    prog->aux->changes_pkt_data)

[Severity: Critical]
This isn't a bug introduced by this patch, but does this check completely
prevent the vulnerability?

It appears that prog->aux->changes_pkt_data might be bypassed if a program
uses bpf_dynptr_write(). Looking at bpf_helper_changes_pkt_data() in
net/core/filter.c, BPF_FUNC_dynptr_write is missing from the switch
statement.

Consequently, the verifier does not set prog->aux->changes_pkt_data during
CFG construction for programs that write via dynptrs. While the verifier
sets a local changes_data flag in check_helper_call() to clear packet
pointers:

kernel/bpf/verifier.c:check_helper_call() {
    ...
        /* this will trigger clear_all_pkt_pointers(), which will
         * invalidate all dynptr slices associated with the skb
         */
        changes_data = true;
    ...
}

It never sets the permanent env->subprog_info[...].changes_pkt_data flag
for this helper.

Could a malicious BPF_PROG_TYPE_SK_SKB program use bpf_dynptr_from_skb() to
get a writable dynptr, and then call bpf_dynptr_write() to invoke
skb_ensure_writable() and free the frag_list? If so, would this trigger the
exact same use-after-free, completely bypassing this attach-time check?

> +		return -EINVAL;
> +
> +	return 0;
> +}

[ ... ]

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