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

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

[Jan-Gerd Tenberge]

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>
---
 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
-- 
2.52.0