Skip to content

Commit e245c00

Browse files
mmhalKernel Patches Daemon
authored andcommitted
bpf: Unconditionally take socket references in lookup helpers
Lookup helpers gate whether to acquire a socket reference on sk_is_refcounted(), a check re-evaluated at release. An established socket refcounted at acquire time can gain SOCK_RCU_FREE via connect(AF_UNSPEC)+listen() before release runs; the release-side re-check then reads sk_is_refcounted() == false and skips the put. The reference leaks. Make acquire and release unconditional and symmetric: always take a reference, always put it. Adapt sk_select_reuseport(). Fixes: 6acc9b4 ("bpf: Add helper to retrieve socket in BPF") Fixes: 64d8529 ("bpf: Allow bpf_map_lookup_elem for SOCKMAP and SOCKHASH") Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://lore.kernel.org/bpf/20260701235552.2B0AA1F00A3F@smtp.kernel.org/ Signed-off-by: Michal Luczaj <mhal@rbox.co> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
1 parent 690b74d commit e245c00

2 files changed

Lines changed: 20 additions & 11 deletions

File tree

net/core/filter.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7032,6 +7032,14 @@ static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
70327032
WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
70337033
sk = NULL;
70347034
}
7035+
7036+
/*
7037+
* Always take a reference, even if the lookup skipped one;
7038+
* bpf_sk_release() always puts one.
7039+
*/
7040+
if (sk && !refcounted && !refcount_inc_not_zero(&sk->sk_refcnt))
7041+
sk = NULL;
7042+
70357043
return sk;
70367044
}
70377045

@@ -7090,11 +7098,16 @@ bpf_sk_lookup_full_sk(struct sock *sk)
70907098
*/
70917099
if (sk2 != sk) {
70927100
sock_gen_put(sk);
7093-
/* Ensure there is no need to bump sk2 refcnt. */
70947101
if (unlikely(sk2 && !sock_flag(sk2, SOCK_RCU_FREE))) {
70957102
WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
70967103
return NULL;
70977104
}
7105+
/*
7106+
* sk2 is RCU-free, but take a reference anyway;
7107+
* bpf_sk_release() puts.
7108+
*/
7109+
if (sk2 && !refcount_inc_not_zero(&sk2->sk_refcnt))
7110+
sk2 = NULL;
70987111
sk = sk2;
70997112
}
71007113

@@ -7279,7 +7292,7 @@ static const struct bpf_func_proto bpf_tc_sk_lookup_udp_proto = {
72797292

72807293
BPF_CALL_1(bpf_sk_release, struct sock *, sk)
72817294
{
7282-
if (sk && sk_is_refcounted(sk))
7295+
if (sk)
72837296
sock_gen_put(sk);
72847297
return 0;
72857298
}
@@ -11571,11 +11584,13 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
1157111584
bool is_sockarray = map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY;
1157211585
struct sock_reuseport *reuse;
1157311586
struct sock *selected_sk;
11574-
int err;
11587+
int err = 0;
1157511588

1157611589
selected_sk = map->ops->map_lookup_elem(map, key);
1157711590
if (!selected_sk)
1157811591
return -ENOENT;
11592+
if (!is_sockarray)
11593+
sock_put(selected_sk);
1157911594

1158011595
reuse = rcu_dereference(selected_sk->sk_reuseport_cb);
1158111596
if (!reuse) {
@@ -11605,13 +11620,7 @@ BPF_CALL_4(sk_select_reuseport, struct sk_reuseport_kern *, reuse_kern,
1160511620
}
1160611621

1160711622
reuse_kern->selected_sk = selected_sk;
11608-
11609-
return 0;
1161011623
error:
11611-
/* Lookup in sock_map can return TCP ESTABLISHED sockets. */
11612-
if (sk_is_refcounted(selected_sk))
11613-
sock_put(selected_sk);
11614-
1161511624
return err;
1161611625
}
1161711626

net/core/sock_map.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ static void *sock_map_lookup(struct bpf_map *map, void *key)
392392
sk = __sock_map_lookup_elem(map, *(u32 *)key);
393393
if (!sk)
394394
return NULL;
395-
if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
395+
if (!refcount_inc_not_zero(&sk->sk_refcnt))
396396
return NULL;
397397
return sk;
398398
}
@@ -1218,7 +1218,7 @@ static void *sock_hash_lookup(struct bpf_map *map, void *key)
12181218
sk = __sock_hash_lookup_elem(map, key);
12191219
if (!sk)
12201220
return NULL;
1221-
if (sk_is_refcounted(sk) && !refcount_inc_not_zero(&sk->sk_refcnt))
1221+
if (!refcount_inc_not_zero(&sk->sk_refcnt))
12221222
return NULL;
12231223
return sk;
12241224
}

0 commit comments

Comments
 (0)