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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 10/8/26 5:56 PM, 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>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>


> ---
>   net/core/sock_map.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index 38df84284328..32ccf4ca3c76 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -26,7 +26,8 @@ struct bpf_stab {
>   
>   /* This mutex is used to
>    *  - protect race between prog/link attach/detach and link prog update, and
> - *  - protect race between releasing and accessing map in bpf_link.
> + *  - protect race between releasing and accessing map in bpf_link, and
> + *  - protect race between map user-reference release and prog/link updates.
>    * A single global mutex lock is used since it is expected contention is low.
>    */
>   static DEFINE_MUTEX(sockmap_mutex);
> @@ -372,7 +373,10 @@ static void sock_map_free(struct bpf_map *map)
>   
>   static void sock_map_release_progs(struct bpf_map *map)
>   {
> -	psock_progs_drop(&container_of(map, struct bpf_stab, map)->progs);
> +	mutex_lock(&sockmap_mutex);
> +	if (!atomic64_read(&map->usercnt))
> +		psock_progs_drop(&container_of(map, struct bpf_stab, map)->progs);
> +	mutex_unlock(&sockmap_mutex);
>   }
>   
>   static struct sock *__sock_map_lookup_elem(struct bpf_map *map, u32 key)
> @@ -1226,7 +1230,10 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
>   
>   static void sock_hash_release_progs(struct bpf_map *map)
>   {
> -	psock_progs_drop(&container_of(map, struct bpf_shtab, map)->progs);
> +	mutex_lock(&sockmap_mutex);
> +	if (!atomic64_read(&map->usercnt))
> +		psock_progs_drop(&container_of(map, struct bpf_shtab, map)->progs);
> +	mutex_unlock(&sockmap_mutex);
>   }
>   
>   BPF_CALL_4(bpf_sock_hash_update, struct bpf_sock_ops_kern *, sops,
> @@ -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);
>   }
>   
>   static int sock_map_link_detach(struct bpf_link *link)
>
> base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc