Skip to content

Commit 5032d07

Browse files
jrfastabborkmann
authored andcommitted
bpf: skmsg, fix psock create on existing kcm/tls port
Before using the psock returned by sk_psock_get() when adding it to a sockmap we need to ensure it is actually a sockmap based psock. Previously we were only checking this after incrementing the reference counter which was an error. This resulted in a slab-out-of-bounds error when the psock was not actually a sockmap type. This moves the check up so the reference counter is only used if it is a sockmap psock. Eric reported the following KASAN BUG, BUG: KASAN: slab-out-of-bounds in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline] BUG: KASAN: slab-out-of-bounds in refcount_inc_not_zero_checked+0x97/0x2f0 lib/refcount.c:120 Read of size 4 at addr ffff88019548be58 by task syz-executor4/22387 CPU: 1 PID: 22387 Comm: syz-executor4 Not tainted 4.19.0-rc7+ #264 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 check_memory_region_inline mm/kasan/kasan.c:260 [inline] check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267 kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272 atomic_read include/asm-generic/atomic-instrumented.h:21 [inline] refcount_inc_not_zero_checked+0x97/0x2f0 lib/refcount.c:120 sk_psock_get include/linux/skmsg.h:379 [inline] sock_map_link.isra.6+0x41f/0xe30 net/core/sock_map.c:178 sock_hash_update_common+0x19b/0x11e0 net/core/sock_map.c:669 sock_hash_update_elem+0x306/0x470 net/core/sock_map.c:738 map_update_elem+0x819/0xdf0 kernel/bpf/syscall.c:818 Signed-off-by: John Fastabend <[email protected]> Reported-by: Eric Dumazet <[email protected]> Fixes: 604326b ("bpf, sockmap: convert to generic sk_msg interface") Signed-off-by: Daniel Borkmann <[email protected]>
1 parent 540fefc commit 5032d07

File tree

2 files changed

+26
-10
lines changed

2 files changed

+26
-10
lines changed

include/linux/skmsg.h

+20-5
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,6 @@ static inline struct sk_psock *sk_psock(const struct sock *sk)
270270
return rcu_dereference_sk_user_data(sk);
271271
}
272272

273-
static inline bool sk_has_psock(struct sock *sk)
274-
{
275-
return sk_psock(sk) != NULL && sk->sk_prot->recvmsg == tcp_bpf_recvmsg;
276-
}
277-
278273
static inline void sk_psock_queue_msg(struct sk_psock *psock,
279274
struct sk_msg *msg)
280275
{
@@ -374,6 +369,26 @@ static inline bool sk_psock_test_state(const struct sk_psock *psock,
374369
return test_bit(bit, &psock->state);
375370
}
376371

372+
static inline struct sk_psock *sk_psock_get_checked(struct sock *sk)
373+
{
374+
struct sk_psock *psock;
375+
376+
rcu_read_lock();
377+
psock = sk_psock(sk);
378+
if (psock) {
379+
if (sk->sk_prot->recvmsg != tcp_bpf_recvmsg) {
380+
psock = ERR_PTR(-EBUSY);
381+
goto out;
382+
}
383+
384+
if (!refcount_inc_not_zero(&psock->refcnt))
385+
psock = ERR_PTR(-EBUSY);
386+
}
387+
out:
388+
rcu_read_unlock();
389+
return psock;
390+
}
391+
377392
static inline struct sk_psock *sk_psock_get(struct sock *sk)
378393
{
379394
struct sk_psock *psock;

net/core/sock_map.c

+6-5
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,13 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs,
175175
}
176176
}
177177

178-
psock = sk_psock_get(sk);
178+
psock = sk_psock_get_checked(sk);
179+
if (IS_ERR(psock)) {
180+
ret = PTR_ERR(psock);
181+
goto out_progs;
182+
}
183+
179184
if (psock) {
180-
if (!sk_has_psock(sk)) {
181-
ret = -EBUSY;
182-
goto out_progs;
183-
}
184185
if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
185186
(skb_progs && READ_ONCE(psock->progs.skb_parser))) {
186187
sk_psock_put(sk, psock);

0 commit comments

Comments
 (0)