All of lore.kernel.org
 help / color / mirror / Atom feed
* Bluetooth: L2cap: use-after-free in l2cap_sock_kill
@ 2023-01-23  9:17 Sungwoo Kim
  2023-02-02  9:05 ` [PATCH] Bluetooth: L2CAP: Fix use-after-free Sungwoo Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Sungwoo Kim @ 2023-01-23  9:17 UTC (permalink / raw)
  Cc: wuruoyu, benquike, daveti, Marcel Holtmann, Johan Hedberg,
	Luiz Augusto von Dentz, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list:BLUETOOTH SUBSYSTEM,
	open list:NETWORKING [GENERAL],
	open list

It is a racy bug between l2cap_chan_timeout() and l2cap_sock_release()
cause by SIGKILL.
Sorry for the less context and no fix here.
For the l2cap_sock.c in the stack trace, please refer this file
for your convenience:
https://gist.github.com/swkim101/5c3b8cb7c7d7172aef23810c9412f323

This is discovered by FuzzBT on top of Syzkaller with Sungwoo Kim (me).
Other contributors for FuzzBT project are Ruoyu Wu(wuruoyu@me.com)
and Hui Peng(benquike@gmail.com).

==================================================================
BUG: KASAN: use-after-free in l2cap_sock_kill (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/sock.h:986 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1281) 
Read of size 8 at addr ffff88800f7f4060 by task l2cap-server/1764
CPU: 0 PID: 1764 Comm: l2cap-server Not tainted 6.1.0-rc2 #129
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/lib/dump_stack.c:105) 
print_address_description+0x7e/0x360 
print_report (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report.c:187 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report.c:389) 
? __virt_addr_valid (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/mmzone.h:1855 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/mm/physaddr.c:65) 
? kasan_complete_mode_report_info (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report_generic.c:104 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report_generic.c:127 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report_generic.c:136) 
? l2cap_sock_kill (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/sock.h:986 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1281) 
kasan_report (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report.c:? /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/report.c:484) 
? l2cap_sock_kill (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/sock.h:986 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1281) 
kasan_check_range (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:85 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:115 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:128 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:159 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:180 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:189) 
__kasan_check_read (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/shadow.c:31) 
l2cap_sock_kill (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/sock.h:986 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1281) 
l2cap_sock_teardown_cb (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/bluetooth/bluetooth.h:304 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1475 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1612) 
l2cap_chan_close (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:885) 
? __kasan_check_write (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/shadow.c:37) 
l2cap_sock_shutdown (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/kcsan-checks.h:231 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/sock.h:2470 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1321 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1377) 
? _raw_write_unlock (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/asm-generic/qrwlock.h:122 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/rwlock_api_smp.h:225 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/locking/spinlock.c:342) 
l2cap_sock_release (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1453) 
sock_close (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/socket.c:1382) 
? sock_mmap (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/socket.c:?) 
__fput (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/fsnotify.h:? /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/fsnotify.h:99 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/fsnotify.h:341 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/fs/file_table.c:306) 
____fput (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/fs/file_table.c:348) 
task_work_run (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/task_work.c:165) 
do_exit (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/exit.c:?) 
do_group_exit (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/exit.c:943) 
? __kasan_check_write (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/shadow.c:37) 
get_signal (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/signal.c:2863) 
? _raw_spin_unlock (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/spinlock_api_smp.h:142 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/locking/spinlock.c:186) 
? finish_task_switch (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./arch/x86/include/asm/current.h:15 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/sched/core.c:5065) 
arch_do_signal_or_restart (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/kernel/signal.c:869) 
exit_to_user_mode_prepare (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/entry/common.c:383) 
syscall_exit_to_user_mode (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./arch/x86/include/asm/current.h:15 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/entry/common.c:261 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/entry/common.c:283 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/entry/common.c:296) 
do_syscall_64 (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/entry/common.c:50 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/entry/common.c:80) 
? sysvec_apic_timer_interrupt (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/kernel/apic/apic.c:1107) 
entry_SYSCALL_64_after_hwframe (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/entry/entry_64.S:120) 
RIP: 0033:0x7f66c14db970
Code: Unable to access opcode bytes at 0x7f66c14db946.

Code starting with the faulting instruction
===========================================
RSP: 002b:00007ffe166a5508 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: 0000000000000013 RBX: 0000000000000013 RCX: 00007f66c14db970
RDX: 0000000000000013 RSI: 00007ffe166a56d0 RDI: 0000000000000002
RBP: 00007ffe166a56d0 R08: 00007f66c1a28440 R09: 0000000000000013
R10: 0000000000000078 R11: 0000000000000246 R12: 0000000000000013
R13: 0000000000000001 R14: 00007f66c179a520 R15: 0000000000000013
 </TASK>
Allocated by task 77:
kasan_set_track (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/common.c:51) 
kasan_save_alloc_info (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:432 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:498) 
__kasan_kmalloc (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/common.c:356) 
__kmalloc (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slab_common.c:943 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slab_common.c:968) 
sk_prot_alloc (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/core/sock.c:2028) 
sk_alloc (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/core/sock.c:2083) 
l2cap_sock_alloc (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1903) 
l2cap_sock_new_connection_cb (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1504) 
l2cap_connect (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:102 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:4277) 
l2cap_bredr_sig_cmd (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:5634 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:5927) 
l2cap_recv_frame (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:7851 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:7919) 
l2cap_recv_acldata (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:8601 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:8631) 
hci_rx_work (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/bluetooth/hci_core.h:1121 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/hci_core.c:3937 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/hci_core.c:4189) 
process_one_work (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2225) 
worker_thread (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:816 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2107 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2159 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2408) 
kthread (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/kthread.c:361) 
ret_from_fork (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/entry/entry_64.S:306) 
Freed by task 52:
kasan_set_track (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/common.c:51) 
kasan_save_free_info (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/generic.c:508) 
____kasan_slab_free (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/slub_def.h:164 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/common.c:214) 
__kasan_slab_free (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/kasan/common.c:244) 
slab_free_freelist_hook (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slub.c:381 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slub.c:1747) 
__kmem_cache_free (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slub.c:3656 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slub.c:3674) 
kfree (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/mm/slab_common.c:1007) 
__sk_destruct (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/cred.h:288 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/core/sock.c:2147) 
__sk_free (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/sock_diag.h:87 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/core/sock.c:2175) 
sk_free (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/instrumented.h:? /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/atomic/atomic-instrumented.h:176 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/refcount.h:272 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/refcount.h:315 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/linux/refcount.h:333 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/core/sock.c:2188) 
l2cap_sock_kill (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/bluetooth/bluetooth.h:286 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1284) 
l2cap_sock_close_cb (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_sock.c:1576) 
l2cap_chan_timeout (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/./include/net/bluetooth/bluetooth.h:296 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/net/bluetooth/l2cap_core.c:462) 
process_one_work (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2225) 
worker_thread (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:816 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2107 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2159 /home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/workqueue.c:2408) 
kthread (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/kernel/kthread.c:361) 
ret_from_fork (/home/sungwoo/fuzzbt/v6.1-rc2-bzimage/arch/x86/entry/entry_64.S:306) 
The buggy address belongs to the object at ffff88800f7f4000
 which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 96 bytes inside of
 1024-byte region [ffff88800f7f4000, ffff88800f7f4400)
The buggy address belongs to the physical page:
page:00000000b8d65c1d refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff88800f7f6800 pfn:0xf7f4
head:00000000b8d65c1d order:2 compound_mapcount:0 compound_pincount:0
flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
raw: 000fffffc0010200 ffffea0000993408 ffffea0000991308 ffff888005841dc0
raw: ffff88800f7f6800 0000000000080002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
 ffff88800f7f3f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff88800f7f3f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff88800f7f4000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                       ^
 ffff88800f7f4080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88800f7f4100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Bluetooth: L2CAP: Fix use-after-free
  2023-01-23  9:17 Bluetooth: L2cap: use-after-free in l2cap_sock_kill Sungwoo Kim
@ 2023-02-02  9:05 ` Sungwoo Kim
  2023-02-02  9:26   ` Eric Dumazet
  2023-02-02  9:33   ` Bluetooth: L2CAP: Fix use-after-free bluez.test.bot
  0 siblings, 2 replies; 9+ messages in thread
From: Sungwoo Kim @ 2023-02-02  9:05 UTC (permalink / raw)
  To: happiness.sung.woo
  Cc: benquike, davem, daveti, edumazet, johan.hedberg, kuba,
	linux-bluetooth, linux-kernel, luiz.dentz, marcel, netdev,
	pabeni, wuruoyu, Sungwoo Kim

Due to the race condition between l2cap_sock_cleanup_listen and
l2cap_sock_close_cb, l2cap_sock_kill can receive already freed sk,
resulting in use-after-free inside l2cap_sock_kill.
This patch prevent this by adding a null check in l2cap_sock_kill.

Context 1:
l2cap_sock_cleanup_listen();
  // context switched
  l2cap_chan_lock(chan);
  l2cap_sock_kill(sk); // <-- sk is already freed below

Context 2:
l2cap_chan_timeout();
  l2cap_chan_lock(chan);
  chan->ops->close(chan);
    l2cap_sock_close_cb()
    l2cap_sock_kill(sk); // <-- sk is freed here
  l2cap_chan_unlock(chan);

Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
 net/bluetooth/l2cap_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index ca8f07f35..657704059 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1245,7 +1245,7 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
  */
 static void l2cap_sock_kill(struct sock *sk)
 {
-	if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
+	if (!sk || !sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
 		return;
 
 	BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Fix use-after-free
  2023-02-02  9:05 ` [PATCH] Bluetooth: L2CAP: Fix use-after-free Sungwoo Kim
@ 2023-02-02  9:26   ` Eric Dumazet
  2023-02-02 12:09     ` Sungwoo Kim
  2023-02-02  9:33   ` Bluetooth: L2CAP: Fix use-after-free bluez.test.bot
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2023-02-02  9:26 UTC (permalink / raw)
  To: Sungwoo Kim
  Cc: happiness.sung.woo, benquike, davem, daveti, johan.hedberg, kuba,
	linux-bluetooth, linux-kernel, luiz.dentz, marcel, netdev,
	pabeni, wuruoyu

On Thu, Feb 2, 2023 at 10:07 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
>
> Due to the race condition between l2cap_sock_cleanup_listen and
> l2cap_sock_close_cb, l2cap_sock_kill can receive already freed sk,
> resulting in use-after-free inside l2cap_sock_kill.
> This patch prevent this by adding a null check in l2cap_sock_kill.
>
> Context 1:
> l2cap_sock_cleanup_listen();
>   // context switched
>   l2cap_chan_lock(chan);
>   l2cap_sock_kill(sk); // <-- sk is already freed below

But sk is used in l2cap_sock_cleanup_listen()
and should not be NULL...

while ((sk = bt_accept_dequeue(parent, NULL))) {
  ...
  l2cap_sock_kill(sk);
  ..
}

It would help if you send us a stack trace ...

>
> Context 2:
> l2cap_chan_timeout();
>   l2cap_chan_lock(chan);
>   chan->ops->close(chan);
>     l2cap_sock_close_cb()
>     l2cap_sock_kill(sk); // <-- sk is freed here
>   l2cap_chan_unlock(chan);
>

Please add a Fixes: tag

> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> ---
>  net/bluetooth/l2cap_sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index ca8f07f35..657704059 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1245,7 +1245,7 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
>   */
>  static void l2cap_sock_kill(struct sock *sk)
>  {
> -       if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> +       if (!sk || !sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
>                 return;
>
>         BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: Bluetooth: L2CAP: Fix use-after-free
  2023-02-02  9:05 ` [PATCH] Bluetooth: L2CAP: Fix use-after-free Sungwoo Kim
  2023-02-02  9:26   ` Eric Dumazet
@ 2023-02-02  9:33   ` bluez.test.bot
  1 sibling, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2023-02-02  9:33 UTC (permalink / raw)
  To: linux-bluetooth, iam

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=718047

---Test result---

Test Summary:
CheckPatch                    PASS      0.66 seconds
GitLint                       PASS      0.24 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      31.44 seconds
CheckAllWarning               PASS      33.94 seconds
CheckSparse                   PASS      38.44 seconds
CheckSmatch                   PASS      106.53 seconds
BuildKernel32                 PASS      29.79 seconds
TestRunnerSetup               PASS      433.73 seconds
TestRunner_l2cap-tester       PASS      16.20 seconds
TestRunner_iso-tester         PASS      16.63 seconds
TestRunner_bnep-tester        PASS      5.44 seconds
TestRunner_mgmt-tester        PASS      108.30 seconds
TestRunner_rfcomm-tester      PASS      8.67 seconds
TestRunner_sco-tester         PASS      8.01 seconds
TestRunner_ioctl-tester       PASS      9.31 seconds
TestRunner_mesh-tester        PASS      6.86 seconds
TestRunner_smp-tester         PASS      7.90 seconds
TestRunner_userchan-tester    PASS      5.67 seconds
IncrementalBuild              PASS      27.87 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Fix use-after-free
  2023-02-02  9:26   ` Eric Dumazet
@ 2023-02-02 12:09     ` Sungwoo Kim
  2023-02-02 12:35       ` Eric Dumazet
  2023-05-26 18:16       ` [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb Sungwoo Kim
  0 siblings, 2 replies; 9+ messages in thread
From: Sungwoo Kim @ 2023-02-02 12:09 UTC (permalink / raw)
  To: edumazet
  Cc: benquike, davem, daveti, happiness.sung.woo, iam, johan.hedberg,
	kuba, linux-bluetooth, linux-kernel, luiz.dentz, marcel, netdev,
	pabeni, wuruoyu

On Thu, Feb 2, 2023 at 4:26 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Feb 2, 2023 at 10:07 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
> >
> > Due to the race condition between l2cap_sock_cleanup_listen and
> > l2cap_sock_close_cb, l2cap_sock_kill can receive already freed sk,
> > resulting in use-after-free inside l2cap_sock_kill.
> > This patch prevent this by adding a null check in l2cap_sock_kill.
> >
> > Context 1:
> > l2cap_sock_cleanup_listen();
> >   // context switched
> >   l2cap_chan_lock(chan);
> >   l2cap_sock_kill(sk); // <-- sk is already freed below
>
> But sk is used in l2cap_sock_cleanup_listen()
> and should not be NULL...
>
> while ((sk = bt_accept_dequeue(parent, NULL))) {
>   ...
>   l2cap_sock_kill(sk);
>   ..
> }
>
> It would help if you send us a stack trace ...

Here is the stack trace and l2cap_sock.c:
https://gist.github.com/swkim101/5c3b8cb7c7d7172aef23810c9412f323

==================================================================
BUG: KASAN: use-after-free in l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281) 
Read of size 8 at addr ffff88800f7f4060 by task l2cap-server/1764
CPU: 0 PID: 1764 Comm: l2cap-server Not tainted 6.1.0-rc2 #129
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (/v6.1-rc2/lib/dump_stack.c:105) 
print_address_description+0x7e/0x360 
print_report (/v6.1-rc2/mm/kasan/report.c:187 /v6.1-rc2/mm/kasan/report.c:389) 
? __virt_addr_valid (/v6.1-rc2/./include/linux/mmzone.h:1855 /v6.1-rc2/arch/x86/mm/physaddr.c:65) 
? kasan_complete_mode_report_info (/v6.1-rc2/mm/kasan/report_generic.c:104 /v6.1-rc2/mm/kasan/report_generic.c:127 /v6.1-rc2/mm/kasan/report_generic.c:136) 
? l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281) 
kasan_report (/v6.1-rc2/mm/kasan/report.c:? /v6.1-rc2/mm/kasan/report.c:484) 
? l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281) 
kasan_check_range (/v6.1-rc2/mm/kasan/generic.c:85 /v6.1-rc2/mm/kasan/generic.c:115 /v6.1-rc2/mm/kasan/generic.c:128 /v6.1-rc2/mm/kasan/generic.c:159 /v6.1-rc2/mm/kasan/generic.c:180 /v6.1-rc2/mm/kasan/generic.c:189) 
__kasan_check_read (/v6.1-rc2/mm/kasan/shadow.c:31) 
l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281) 
l2cap_sock_teardown_cb (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:304 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1475 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1612) 
l2cap_chan_close (/v6.1-rc2/net/bluetooth/l2cap_core.c:885) 
? __kasan_check_write (/v6.1-rc2/mm/kasan/shadow.c:37) 
l2cap_sock_shutdown (/v6.1-rc2/./include/linux/kcsan-checks.h:231 /v6.1-rc2/./include/net/sock.h:2470 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1321 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1377) 
? _raw_write_unlock (/v6.1-rc2/./include/asm-generic/qrwlock.h:122 /v6.1-rc2/./include/linux/rwlock_api_smp.h:225 /v6.1-rc2/kernel/locking/spinlock.c:342) 
l2cap_sock_release (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1453) 
sock_close (/v6.1-rc2/net/socket.c:1382) 
? sock_mmap (/v6.1-rc2/net/socket.c:?) 
__fput (/v6.1-rc2/./include/linux/fsnotify.h:? /v6.1-rc2/./include/linux/fsnotify.h:99 /v6.1-rc2/./include/linux/fsnotify.h:341 /v6.1-rc2/fs/file_table.c:306) 
____fput (/v6.1-rc2/fs/file_table.c:348) 
task_work_run (/v6.1-rc2/kernel/task_work.c:165) 
do_exit (/v6.1-rc2/kernel/exit.c:?) 
do_group_exit (/v6.1-rc2/kernel/exit.c:943) 
? __kasan_check_write (/v6.1-rc2/mm/kasan/shadow.c:37) 
get_signal (/v6.1-rc2/kernel/signal.c:2863) 
? _raw_spin_unlock (/v6.1-rc2/./include/linux/spinlock_api_smp.h:142 /v6.1-rc2/kernel/locking/spinlock.c:186) 
? finish_task_switch (/v6.1-rc2/./arch/x86/include/asm/current.h:15 /v6.1-rc2/kernel/sched/core.c:5065) 
arch_do_signal_or_restart (/v6.1-rc2/arch/x86/kernel/signal.c:869) 
exit_to_user_mode_prepare (/v6.1-rc2/kernel/entry/common.c:383) 
syscall_exit_to_user_mode (/v6.1-rc2/./arch/x86/include/asm/current.h:15 /v6.1-rc2/kernel/entry/common.c:261 /v6.1-rc2/kernel/entry/common.c:283 /v6.1-rc2/kernel/entry/common.c:296) 
do_syscall_64 (/v6.1-rc2/arch/x86/entry/common.c:50 /v6.1-rc2/arch/x86/entry/common.c:80) 
? sysvec_apic_timer_interrupt (/v6.1-rc2/arch/x86/kernel/apic/apic.c:1107) 
entry_SYSCALL_64_after_hwframe (/v6.1-rc2/arch/x86/entry/entry_64.S:120) 
RIP: 0033:0x7f66c14db970
Code: Unable to access opcode bytes at 0x7f66c14db946.

Code starting with the faulting instruction
===========================================
RSP: 002b:00007ffe166a5508 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: 0000000000000013 RBX: 0000000000000013 RCX: 00007f66c14db970
RDX: 0000000000000013 RSI: 00007ffe166a56d0 RDI: 0000000000000002
RBP: 00007ffe166a56d0 R08: 00007f66c1a28440 R09: 0000000000000013
R10: 0000000000000078 R11: 0000000000000246 R12: 0000000000000013
R13: 0000000000000001 R14: 00007f66c179a520 R15: 0000000000000013
 </TASK>
Allocated by task 77:
kasan_set_track (/v6.1-rc2/mm/kasan/common.c:51) 
kasan_save_alloc_info (/v6.1-rc2/mm/kasan/generic.c:432 /v6.1-rc2/mm/kasan/generic.c:498) 
__kasan_kmalloc (/v6.1-rc2/mm/kasan/common.c:356) 
__kmalloc (/v6.1-rc2/mm/slab_common.c:943 /v6.1-rc2/mm/slab_common.c:968) 
sk_prot_alloc (/v6.1-rc2/net/core/sock.c:2028) 
sk_alloc (/v6.1-rc2/net/core/sock.c:2083) 
l2cap_sock_alloc (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1903) 
l2cap_sock_new_connection_cb (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1504) 
l2cap_connect (/v6.1-rc2/net/bluetooth/l2cap_core.c:102 /v6.1-rc2/net/bluetooth/l2cap_core.c:4277) 
l2cap_bredr_sig_cmd (/v6.1-rc2/net/bluetooth/l2cap_core.c:5634 /v6.1-rc2/net/bluetooth/l2cap_core.c:5927) 
l2cap_recv_frame (/v6.1-rc2/net/bluetooth/l2cap_core.c:7851 /v6.1-rc2/net/bluetooth/l2cap_core.c:7919) 
l2cap_recv_acldata (/v6.1-rc2/net/bluetooth/l2cap_core.c:8601 /v6.1-rc2/net/bluetooth/l2cap_core.c:8631) 
hci_rx_work (/v6.1-rc2/./include/net/bluetooth/hci_core.h:1121 /v6.1-rc2/net/bluetooth/hci_core.c:3937 /v6.1-rc2/net/bluetooth/hci_core.c:4189) 
process_one_work (/v6.1-rc2/kernel/workqueue.c:2225) 
worker_thread (/v6.1-rc2/kernel/workqueue.c:816 /v6.1-rc2/kernel/workqueue.c:2107 /v6.1-rc2/kernel/workqueue.c:2159 /v6.1-rc2/kernel/workqueue.c:2408) 
kthread (/v6.1-rc2/kernel/kthread.c:361) 
ret_from_fork (/v6.1-rc2/arch/x86/entry/entry_64.S:306) 
Freed by task 52:
kasan_set_track (/v6.1-rc2/mm/kasan/common.c:51) 
kasan_save_free_info (/v6.1-rc2/mm/kasan/generic.c:508) 
____kasan_slab_free (/v6.1-rc2/./include/linux/slub_def.h:164 /v6.1-rc2/mm/kasan/common.c:214) 
__kasan_slab_free (/v6.1-rc2/mm/kasan/common.c:244) 
slab_free_freelist_hook (/v6.1-rc2/mm/slub.c:381 /v6.1-rc2/mm/slub.c:1747) 
__kmem_cache_free (/v6.1-rc2/mm/slub.c:3656 /v6.1-rc2/mm/slub.c:3674) 
kfree (/v6.1-rc2/mm/slab_common.c:1007) 
__sk_destruct (/v6.1-rc2/./include/linux/cred.h:288 /v6.1-rc2/net/core/sock.c:2147) 
__sk_free (/v6.1-rc2/./include/linux/sock_diag.h:87 /v6.1-rc2/net/core/sock.c:2175) 
sk_free (/v6.1-rc2/./include/linux/instrumented.h:? /v6.1-rc2/./include/linux/atomic/atomic-instrumented.h:176 /v6.1-rc2/./include/linux/refcount.h:272 /v6.1-rc2/./include/linux/refcount.h:315 /v6.1-rc2/./include/linux/refcount.h:333 /v6.1-rc2/net/core/sock.c:2188) 
l2cap_sock_kill (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:286 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1284) 
l2cap_sock_close_cb (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1576) 
l2cap_chan_timeout (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:296 /v6.1-rc2/net/bluetooth/l2cap_core.c:462) 
process_one_work (/v6.1-rc2/kernel/workqueue.c:2225) 
worker_thread (/v6.1-rc2/kernel/workqueue.c:816 /v6.1-rc2/kernel/workqueue.c:2107 /v6.1-rc2/kernel/workqueue.c:2159 /v6.1-rc2/kernel/workqueue.c:2408) 
kthread (/v6.1-rc2/kernel/kthread.c:361) 
ret_from_fork (/v6.1-rc2/arch/x86/entry/entry_64.S:306) 
The buggy address belongs to the object at ffff88800f7f4000
 which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 96 bytes inside of
 1024-byte region [ffff88800f7f4000, ffff88800f7f4400)
The buggy address belongs to the physical page:
page:00000000b8d65c1d refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff88800f7f6800 pfn:0xf7f4
head:00000000b8d65c1d order:2 compound_mapcount:0 compound_pincount:0
flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
raw: 000fffffc0010200 ffffea0000993408 ffffea0000991308 ffff888005841dc0
raw: ffff88800f7f6800 0000000000080002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
 ffff88800f7f3f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff88800f7f3f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff88800f7f4000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                       ^
 ffff88800f7f4080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88800f7f4100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

> >
> > Context 2:
> > l2cap_chan_timeout();
> >   l2cap_chan_lock(chan);
> >   chan->ops->close(chan);
> >     l2cap_sock_close_cb()
> >     l2cap_sock_kill(sk); // <-- sk is freed here
> >   l2cap_chan_unlock(chan);
> >
>
> Please add a Fixes: tag

Fixes: 6c08fc896b60 ("Bluetooth: Fix refcount use-after-free issue")
> > Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> > ---
> >  net/bluetooth/l2cap_sock.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index ca8f07f35..657704059 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1245,7 +1245,7 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> >   */
> >  static void l2cap_sock_kill(struct sock *sk)
> >  {
> > -       if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> > +       if (!sk || !sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> >                 return;
> >
> >         BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Fix use-after-free
  2023-02-02 12:09     ` Sungwoo Kim
@ 2023-02-02 12:35       ` Eric Dumazet
  2023-05-26 18:16       ` [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb Sungwoo Kim
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2023-02-02 12:35 UTC (permalink / raw)
  To: Sungwoo Kim
  Cc: benquike, davem, daveti, happiness.sung.woo, johan.hedberg, kuba,
	linux-bluetooth, linux-kernel, luiz.dentz, marcel, netdev,
	pabeni, wuruoyu

On Thu, Feb 2, 2023 at 1:09 PM Sungwoo Kim <iam@sung-woo.kim> wrote:
>
> On Thu, Feb 2, 2023 at 4:26 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Feb 2, 2023 at 10:07 AM Sungwoo Kim <iam@sung-woo.kim> wrote:
> > >
> > > Due to the race condition between l2cap_sock_cleanup_listen and
> > > l2cap_sock_close_cb, l2cap_sock_kill can receive already freed sk,
> > > resulting in use-after-free inside l2cap_sock_kill.
> > > This patch prevent this by adding a null check in l2cap_sock_kill.
> > >
> > > Context 1:
> > > l2cap_sock_cleanup_listen();
> > >   // context switched
> > >   l2cap_chan_lock(chan);
> > >   l2cap_sock_kill(sk); // <-- sk is already freed below
> >
> > But sk is used in l2cap_sock_cleanup_listen()
> > and should not be NULL...
> >
> > while ((sk = bt_accept_dequeue(parent, NULL))) {
> >   ...
> >   l2cap_sock_kill(sk);
> >   ..
> > }
> >
> > It would help if you send us a stack trace ...
>
> Here is the stack trace and l2cap_sock.c:
> https://gist.github.com/swkim101/5c3b8cb7c7d7172aef23810c9412f323
>
> ==================================================================
> BUG: KASAN: use-after-free in l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281)
> Read of size 8 at addr ffff88800f7f4060 by task l2cap-server/1764
> CPU: 0 PID: 1764 Comm: l2cap-server Not tainted 6.1.0-rc2 #129
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> Call Trace:
>  <TASK>
> dump_stack_lvl (/v6.1-rc2/lib/dump_stack.c:105)
> print_address_description+0x7e/0x360
> print_report (/v6.1-rc2/mm/kasan/report.c:187 /v6.1-rc2/mm/kasan/report.c:389)
> ? __virt_addr_valid (/v6.1-rc2/./include/linux/mmzone.h:1855 /v6.1-rc2/arch/x86/mm/physaddr.c:65)
> ? kasan_complete_mode_report_info (/v6.1-rc2/mm/kasan/report_generic.c:104 /v6.1-rc2/mm/kasan/report_generic.c:127 /v6.1-rc2/mm/kasan/report_generic.c:136)
> ? l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281)
> kasan_report (/v6.1-rc2/mm/kasan/report.c:? /v6.1-rc2/mm/kasan/report.c:484)
> ? l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281)
> kasan_check_range (/v6.1-rc2/mm/kasan/generic.c:85 /v6.1-rc2/mm/kasan/generic.c:115 /v6.1-rc2/mm/kasan/generic.c:128 /v6.1-rc2/mm/kasan/generic.c:159 /v6.1-rc2/mm/kasan/generic.c:180 /v6.1-rc2/mm/kasan/generic.c:189)
> __kasan_check_read (/v6.1-rc2/mm/kasan/shadow.c:31)
> l2cap_sock_kill (/v6.1-rc2/./include/net/sock.h:986 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1281)
> l2cap_sock_teardown_cb (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:304 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1475 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1612)
> l2cap_chan_close (/v6.1-rc2/net/bluetooth/l2cap_core.c:885)
> ? __kasan_check_write (/v6.1-rc2/mm/kasan/shadow.c:37)
> l2cap_sock_shutdown (/v6.1-rc2/./include/linux/kcsan-checks.h:231 /v6.1-rc2/./include/net/sock.h:2470 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1321 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1377)
> ? _raw_write_unlock (/v6.1-rc2/./include/asm-generic/qrwlock.h:122 /v6.1-rc2/./include/linux/rwlock_api_smp.h:225 /v6.1-rc2/kernel/locking/spinlock.c:342)
> l2cap_sock_release (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1453)
> sock_close (/v6.1-rc2/net/socket.c:1382)
> ? sock_mmap (/v6.1-rc2/net/socket.c:?)
> __fput (/v6.1-rc2/./include/linux/fsnotify.h:? /v6.1-rc2/./include/linux/fsnotify.h:99 /v6.1-rc2/./include/linux/fsnotify.h:341 /v6.1-rc2/fs/file_table.c:306)
> ____fput (/v6.1-rc2/fs/file_table.c:348)
> task_work_run (/v6.1-rc2/kernel/task_work.c:165)
> do_exit (/v6.1-rc2/kernel/exit.c:?)
> do_group_exit (/v6.1-rc2/kernel/exit.c:943)

OK, now compare this trace with what you put in your changelog...

Very different problem.

Context 1:
l2cap_sock_cleanup_listen();
  // context switched
  l2cap_chan_lock(chan);
  l2cap_sock_kill(sk); // <-- sk is already freed below


On current linux tree, all l2cap_sock_kill() callers already checked sk != NULL

Do you have a repro ?

> ? __kasan_check_write (/v6.1-rc2/mm/kasan/shadow.c:37)
> get_signal (/v6.1-rc2/kernel/signal.c:2863)
> ? _raw_spin_unlock (/v6.1-rc2/./include/linux/spinlock_api_smp.h:142 /v6.1-rc2/kernel/locking/spinlock.c:186)
> ? finish_task_switch (/v6.1-rc2/./arch/x86/include/asm/current.h:15 /v6.1-rc2/kernel/sched/core.c:5065)
> arch_do_signal_or_restart (/v6.1-rc2/arch/x86/kernel/signal.c:869)
> exit_to_user_mode_prepare (/v6.1-rc2/kernel/entry/common.c:383)
> syscall_exit_to_user_mode (/v6.1-rc2/./arch/x86/include/asm/current.h:15 /v6.1-rc2/kernel/entry/common.c:261 /v6.1-rc2/kernel/entry/common.c:283 /v6.1-rc2/kernel/entry/common.c:296)
> do_syscall_64 (/v6.1-rc2/arch/x86/entry/common.c:50 /v6.1-rc2/arch/x86/entry/common.c:80)
> ? sysvec_apic_timer_interrupt (/v6.1-rc2/arch/x86/kernel/apic/apic.c:1107)
> entry_SYSCALL_64_after_hwframe (/v6.1-rc2/arch/x86/entry/entry_64.S:120)
> RIP: 0033:0x7f66c14db970
> Code: Unable to access opcode bytes at 0x7f66c14db946.
>
> Code starting with the faulting instruction
> ===========================================
> RSP: 002b:00007ffe166a5508 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: 0000000000000013 RBX: 0000000000000013 RCX: 00007f66c14db970
> RDX: 0000000000000013 RSI: 00007ffe166a56d0 RDI: 0000000000000002
> RBP: 00007ffe166a56d0 R08: 00007f66c1a28440 R09: 0000000000000013
> R10: 0000000000000078 R11: 0000000000000246 R12: 0000000000000013
> R13: 0000000000000001 R14: 00007f66c179a520 R15: 0000000000000013
>  </TASK>
> Allocated by task 77:
> kasan_set_track (/v6.1-rc2/mm/kasan/common.c:51)
> kasan_save_alloc_info (/v6.1-rc2/mm/kasan/generic.c:432 /v6.1-rc2/mm/kasan/generic.c:498)
> __kasan_kmalloc (/v6.1-rc2/mm/kasan/common.c:356)
> __kmalloc (/v6.1-rc2/mm/slab_common.c:943 /v6.1-rc2/mm/slab_common.c:968)
> sk_prot_alloc (/v6.1-rc2/net/core/sock.c:2028)
> sk_alloc (/v6.1-rc2/net/core/sock.c:2083)
> l2cap_sock_alloc (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1903)
> l2cap_sock_new_connection_cb (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1504)
> l2cap_connect (/v6.1-rc2/net/bluetooth/l2cap_core.c:102 /v6.1-rc2/net/bluetooth/l2cap_core.c:4277)
> l2cap_bredr_sig_cmd (/v6.1-rc2/net/bluetooth/l2cap_core.c:5634 /v6.1-rc2/net/bluetooth/l2cap_core.c:5927)
> l2cap_recv_frame (/v6.1-rc2/net/bluetooth/l2cap_core.c:7851 /v6.1-rc2/net/bluetooth/l2cap_core.c:7919)
> l2cap_recv_acldata (/v6.1-rc2/net/bluetooth/l2cap_core.c:8601 /v6.1-rc2/net/bluetooth/l2cap_core.c:8631)
> hci_rx_work (/v6.1-rc2/./include/net/bluetooth/hci_core.h:1121 /v6.1-rc2/net/bluetooth/hci_core.c:3937 /v6.1-rc2/net/bluetooth/hci_core.c:4189)
> process_one_work (/v6.1-rc2/kernel/workqueue.c:2225)
> worker_thread (/v6.1-rc2/kernel/workqueue.c:816 /v6.1-rc2/kernel/workqueue.c:2107 /v6.1-rc2/kernel/workqueue.c:2159 /v6.1-rc2/kernel/workqueue.c:2408)
> kthread (/v6.1-rc2/kernel/kthread.c:361)
> ret_from_fork (/v6.1-rc2/arch/x86/entry/entry_64.S:306)
> Freed by task 52:
> kasan_set_track (/v6.1-rc2/mm/kasan/common.c:51)
> kasan_save_free_info (/v6.1-rc2/mm/kasan/generic.c:508)
> ____kasan_slab_free (/v6.1-rc2/./include/linux/slub_def.h:164 /v6.1-rc2/mm/kasan/common.c:214)
> __kasan_slab_free (/v6.1-rc2/mm/kasan/common.c:244)
> slab_free_freelist_hook (/v6.1-rc2/mm/slub.c:381 /v6.1-rc2/mm/slub.c:1747)
> __kmem_cache_free (/v6.1-rc2/mm/slub.c:3656 /v6.1-rc2/mm/slub.c:3674)
> kfree (/v6.1-rc2/mm/slab_common.c:1007)
> __sk_destruct (/v6.1-rc2/./include/linux/cred.h:288 /v6.1-rc2/net/core/sock.c:2147)
> __sk_free (/v6.1-rc2/./include/linux/sock_diag.h:87 /v6.1-rc2/net/core/sock.c:2175)
> sk_free (/v6.1-rc2/./include/linux/instrumented.h:? /v6.1-rc2/./include/linux/atomic/atomic-instrumented.h:176 /v6.1-rc2/./include/linux/refcount.h:272 /v6.1-rc2/./include/linux/refcount.h:315 /v6.1-rc2/./include/linux/refcount.h:333 /v6.1-rc2/net/core/sock.c:2188)
> l2cap_sock_kill (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:286 /v6.1-rc2/net/bluetooth/l2cap_sock.c:1284)
> l2cap_sock_close_cb (/v6.1-rc2/net/bluetooth/l2cap_sock.c:1576)
> l2cap_chan_timeout (/v6.1-rc2/./include/net/bluetooth/bluetooth.h:296 /v6.1-rc2/net/bluetooth/l2cap_core.c:462)
> process_one_work (/v6.1-rc2/kernel/workqueue.c:2225)
> worker_thread (/v6.1-rc2/kernel/workqueue.c:816 /v6.1-rc2/kernel/workqueue.c:2107 /v6.1-rc2/kernel/workqueue.c:2159 /v6.1-rc2/kernel/workqueue.c:2408)
> kthread (/v6.1-rc2/kernel/kthread.c:361)
> ret_from_fork (/v6.1-rc2/arch/x86/entry/entry_64.S:306)
> The buggy address belongs to the object at ffff88800f7f4000
>  which belongs to the cache kmalloc-1k of size 1024
> The buggy address is located 96 bytes inside of
>  1024-byte region [ffff88800f7f4000, ffff88800f7f4400)
> The buggy address belongs to the physical page:
> page:00000000b8d65c1d refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff88800f7f6800 pfn:0xf7f4
> head:00000000b8d65c1d order:2 compound_mapcount:0 compound_pincount:0
> flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
> raw: 000fffffc0010200 ffffea0000993408 ffffea0000991308 ffff888005841dc0
> raw: ffff88800f7f6800 0000000000080002 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
> Memory state around the buggy address:
>  ffff88800f7f3f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>  ffff88800f7f3f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> >ffff88800f7f4000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                                                        ^
>  ffff88800f7f4080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ffff88800f7f4100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
> > >
> > > Context 2:
> > > l2cap_chan_timeout();
> > >   l2cap_chan_lock(chan);
> > >   chan->ops->close(chan);
> > >     l2cap_sock_close_cb()
> > >     l2cap_sock_kill(sk); // <-- sk is freed here
> > >   l2cap_chan_unlock(chan);
> > >
> >
> > Please add a Fixes: tag
>
> Fixes: 6c08fc896b60 ("Bluetooth: Fix refcount use-after-free issue")
> > > Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
> > > ---
> > >  net/bluetooth/l2cap_sock.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > > index ca8f07f35..657704059 100644
> > > --- a/net/bluetooth/l2cap_sock.c
> > > +++ b/net/bluetooth/l2cap_sock.c
> > > @@ -1245,7 +1245,7 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
> > >   */
> > >  static void l2cap_sock_kill(struct sock *sk)
> > >  {
> > > -       if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> > > +       if (!sk || !sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> > >                 return;
> > >
> > >         BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
> > > --
> > > 2.25.1
> > >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb
  2023-02-02 12:09     ` Sungwoo Kim
  2023-02-02 12:35       ` Eric Dumazet
@ 2023-05-26 18:16       ` Sungwoo Kim
  2023-05-26 18:57         ` bluez.test.bot
  2023-05-27 12:59         ` [PATCH] " Simon Horman
  1 sibling, 2 replies; 9+ messages in thread
From: Sungwoo Kim @ 2023-05-26 18:16 UTC (permalink / raw)
  To: iam
  Cc: benquike, davem, daveti, edumazet, happiness.sung.woo,
	johan.hedberg, kuba, linux-bluetooth, linux-kernel, luiz.dentz,
	marcel, netdev, pabeni, wuruoyu

>    net/bluetooth/l2cap_sock.c: In function 'l2cap_sock_release':
> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]

Fix this error

>     1418 |         l2cap_sock_cleanup_listen(sk);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
>          |         l2cap_sock_listen
>    net/bluetooth/l2cap_sock.c: At top level:
> >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'
>     1436 | static void l2cap_sock_cleanup_listen(struct sock *parent)
>          |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration
>    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'
>     1418 |         l2cap_sock_cleanup_listen(sk);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors

Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
---
 net/bluetooth/l2cap_sock.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index eebe25610..3818e11a8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -46,6 +46,7 @@ static const struct proto_ops l2cap_sock_ops;
 static void l2cap_sock_init(struct sock *sk, struct sock *parent);
 static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
 				     int proto, gfp_t prio, int kern);
+static void l2cap_sock_cleanup_listen(struct sock *parent);
 
 bool l2cap_is_socket(struct socket *sock)
 {
@@ -1414,7 +1415,8 @@ static int l2cap_sock_release(struct socket *sock)
 
 	if (!sk)
 		return 0;
-
+		
+	l2cap_sock_cleanup_listen(sk);
 	bt_sock_unlink(&l2cap_sk_list, sk);
 
 	err = l2cap_sock_shutdown(sock, SHUT_RDWR);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb
  2023-05-26 18:16       ` [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb Sungwoo Kim
@ 2023-05-26 18:57         ` bluez.test.bot
  2023-05-27 12:59         ` [PATCH] " Simon Horman
  1 sibling, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2023-05-26 18:57 UTC (permalink / raw)
  To: linux-bluetooth, iam

[-- Attachment #1: Type: text/plain, Size: 3738 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=751481

---Test result---

Test Summary:
CheckPatch                    FAIL      0.81 seconds
GitLint                       FAIL      0.54 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      31.67 seconds
CheckAllWarning               PASS      34.90 seconds
CheckSparse                   PASS      39.38 seconds
CheckSmatch                   PASS      110.87 seconds
BuildKernel32                 PASS      30.74 seconds
TestRunnerSetup               PASS      441.52 seconds
TestRunner_l2cap-tester       PASS      16.65 seconds
TestRunner_iso-tester         PASS      21.28 seconds
TestRunner_bnep-tester        PASS      5.47 seconds
TestRunner_mgmt-tester        PASS      110.94 seconds
TestRunner_rfcomm-tester      PASS      8.76 seconds
TestRunner_sco-tester         PASS      8.05 seconds
TestRunner_ioctl-tester       PASS      9.27 seconds
TestRunner_mesh-tester        PASS      6.87 seconds
TestRunner_smp-tester         PASS      7.98 seconds
TestRunner_userchan-tester    PASS      5.72 seconds
IncrementalBuild              PASS      29.11 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#74: 
> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]

ERROR: trailing whitespace
#113: FILE: net/bluetooth/l2cap_sock.c:1418:
+^I^I$

total: 1 errors, 1 warnings, 0 checks, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/src/13257259.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
4: B1 Line exceeds max length (182>80): "> >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]"
12: B1 Line exceeds max length (127>80): "> >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'"
15: B1 Line exceeds max length (128>80): "> >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration"
16: B1 Line exceeds max length (138>80): ">    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'"


---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb
  2023-05-26 18:16       ` [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb Sungwoo Kim
  2023-05-26 18:57         ` bluez.test.bot
@ 2023-05-27 12:59         ` Simon Horman
  1 sibling, 0 replies; 9+ messages in thread
From: Simon Horman @ 2023-05-27 12:59 UTC (permalink / raw)
  To: Sungwoo Kim
  Cc: benquike, davem, daveti, edumazet, happiness.sung.woo,
	johan.hedberg, kuba, linux-bluetooth, linux-kernel, luiz.dentz,
	marcel, netdev, pabeni, wuruoyu

On Fri, May 26, 2023 at 02:16:48PM -0400, Sungwoo Kim wrote:
> >    net/bluetooth/l2cap_sock.c: In function 'l2cap_sock_release':
> > >> net/bluetooth/l2cap_sock.c:1418:9: error: implicit declaration of function 'l2cap_sock_cleanup_listen'; did you mean 'l2cap_sock_listen'? [-Werror=implicit-function-declaration]
> 
> Fix this error
> 
> >     1418 |         l2cap_sock_cleanup_listen(sk);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
> >          |         l2cap_sock_listen
> >    net/bluetooth/l2cap_sock.c: At top level:
> > >> net/bluetooth/l2cap_sock.c:1436:13: warning: conflicting types for 'l2cap_sock_cleanup_listen'; have 'void(struct sock *)'
> >     1436 | static void l2cap_sock_cleanup_listen(struct sock *parent)
> >          |             ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >> net/bluetooth/l2cap_sock.c:1436:13: error: static declaration of 'l2cap_sock_cleanup_listen' follows non-static declaration
> >    net/bluetooth/l2cap_sock.c:1418:9: note: previous implicit declaration of 'l2cap_sock_cleanup_listen' with type 'void(struct sock *)'
> >     1418 |         l2cap_sock_cleanup_listen(sk);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
> >    cc1: some warnings being treated as errors
> 
> Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>

Hi,

I am confused about why this error occurs.
In bluetooth-next [1] I see that l2cap_sock_cleanup_listen() is defined
on line  1435 of l2cap_sock.c. And then used on line 1574.
So there should be no need for a forward declaration.

[1] a088d769ef3a ("Bluetooth: L2CAP: Fix use-after-free")

> ---
>  net/bluetooth/l2cap_sock.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index eebe25610..3818e11a8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -46,6 +46,7 @@ static const struct proto_ops l2cap_sock_ops;
>  static void l2cap_sock_init(struct sock *sk, struct sock *parent);
>  static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
>  				     int proto, gfp_t prio, int kern);
> +static void l2cap_sock_cleanup_listen(struct sock *parent);
>  
>  bool l2cap_is_socket(struct socket *sock)
>  {
> @@ -1414,7 +1415,8 @@ static int l2cap_sock_release(struct socket *sock)
>  
>  	if (!sk)
>  		return 0;
> -
> +		

nit: The white-space on the line above was correct (no white-space)
     Now there are trailing tabs.

> +	l2cap_sock_cleanup_listen(sk);

This change may match the patch subject
but seems unrelated to the patch description.

>  	bt_sock_unlink(&l2cap_sk_list, sk);
>  
>  	err = l2cap_sock_shutdown(sock, SHUT_RDWR);
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-05-27 12:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-23  9:17 Bluetooth: L2cap: use-after-free in l2cap_sock_kill Sungwoo Kim
2023-02-02  9:05 ` [PATCH] Bluetooth: L2CAP: Fix use-after-free Sungwoo Kim
2023-02-02  9:26   ` Eric Dumazet
2023-02-02 12:09     ` Sungwoo Kim
2023-02-02 12:35       ` Eric Dumazet
2023-05-26 18:16       ` [PATCH] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_ready_cb Sungwoo Kim
2023-05-26 18:57         ` bluez.test.bot
2023-05-27 12:59         ` [PATCH] " Simon Horman
2023-02-02  9:33   ` Bluetooth: L2CAP: Fix use-after-free bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.