Re: [PATCH bpf] bpf, sockmap: Fix UAF when map user reference is revived

[Jakub Sitnicki <jakub@cloudflare.com>]

On Thu, Oct 08, 2026 at 11:56 AM +02, Jan-Gerd Tenberge wrote:
> bpf_map_put_uref() invokes map_release_uref after usercnt reaches zero,
> before bpf_map_put() drops the map reference. BPF_MAP_GET_FD_BY_ID can
> find the map during that callback and raise usercnt again.
>
> The sockmap and sockhash release callbacks unconditionally drop programs
> without taking sockmap_mutex. A concurrent attach through the revived fd
> can therefore publish a new program before the stale callback drops it.
>
> For a bpf_link attachment, this leaves plink set while pprog is NULL, so
> link release warns and leaves the attachment slot unusable. The callback
> can also drop the program between sock_map_prog_update() and the later
> bpf_prog_inc(). If the program fd is closed concurrently, this removes
> the last reference and schedules an RCU free. The subsequent increment
> then resurrects a zero reference and leaves link->prog pointing to memory
> that will be freed. A KASAN reproducer triggers a slab-use-after-free when
> reading information from that link.
>
> Triggering the race requires CAP_SYS_ADMIN in the initial user namespace,
> because reviving the map uses BPF_MAP_GET_FD_BY_ID. A deterministic KASAN
> reproducer is available privately on request. It was used to verify the
> UAF before this change and its absence afterwards.
>
> Serialize the release callbacks with program updates using sockmap_mutex
> and recheck usercnt under the mutex. If the map was revived, leave its
> programs intact. Move the map user-reference put in link release outside
> the mutex to avoid recursively taking sockmap_mutex when it drops the last
> user reference.
>
> Fixes: 699c23f02c65 ("bpf: Add bpf_link support for sk_msg and sk_skb progs")
> Assisted-by: LLM
> Cc: stable@vger.kernel.org
> Signed-off-by: Jan-Gerd Tenberge <janten@gmail.com>
> ---

Please include KASAN report in the description.
That helps others find that the issue they hit has been fixed.

[...]

> @@ -1743,6 +1750,7 @@ struct sockmap_link {
>  static void sock_map_link_release(struct bpf_link *link)
>  {
>  	struct sockmap_link *sockmap_link = container_of(link, struct sockmap_link, link);
> +	struct bpf_map *map = NULL;
>  
>  	mutex_lock(&sockmap_mutex);
>  	if (!sockmap_link->map)
> @@ -1751,10 +1759,12 @@ static void sock_map_link_release(struct bpf_link *link)
>  	WARN_ON_ONCE(sock_map_prog_update(sockmap_link->map, NULL, link->prog, link,
>  					  link->attach_type));
>  
> -	bpf_map_put_with_uref(sockmap_link->map);
> +	map = sockmap_link->map;
>  	sockmap_link->map = NULL;
>  out:
>  	mutex_unlock(&sockmap_mutex);
> +	if (map)
> +		bpf_map_put_with_uref(map);
>  }

In sock_map_link_release map->usercnt is now modifed without
sockmap_mutex held. Is that a problem?