linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout
@ 2023-11-16 11:20 syzbot
  0 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-16 11:20 UTC (permalink / raw)
  To: johan.hedberg, linux-bluetooth, linux-kernel, luiz.dentz, marcel,
	syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    8de1e7afcc1c Merge branch 'for-next/core' into for-kernelci
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=1126f190e80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=3e6feaeda5dcbc27
dashboard link: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm64
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=122a2560e80000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=136e08df680000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/0f00907f9764/disk-8de1e7af.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/0502fe78c60d/vmlinux-8de1e7af.xz
kernel image: https://storage.googleapis.com/syzbot-assets/192135168cc0/Image-8de1e7af.gz.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: slab-use-after-free in instrument_atomic_read_write include/linux/instrumented.h:96 [inline]
BUG: KASAN: slab-use-after-free in atomic_fetch_add_relaxed include/linux/atomic/atomic-instrumented.h:252 [inline]
BUG: KASAN: slab-use-after-free in __refcount_add include/linux/refcount.h:193 [inline]
BUG: KASAN: slab-use-after-free in __refcount_inc include/linux/refcount.h:250 [inline]
BUG: KASAN: slab-use-after-free in refcount_inc include/linux/refcount.h:267 [inline]
BUG: KASAN: slab-use-after-free in sock_hold include/net/sock.h:777 [inline]
BUG: KASAN: slab-use-after-free in sco_sock_timeout+0x64/0x25c net/bluetooth/sco.c:88
Write of size 4 at addr ffff0000dba59080 by task kworker/0:1/10

CPU: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.6.0-rc7-syzkaller-g8de1e7afcc1c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
Workqueue: events sco_sock_timeout
Call trace:
 dump_backtrace+0x1b8/0x1e4 arch/arm64/kernel/stacktrace.c:233
 show_stack+0x2c/0x44 arch/arm64/kernel/stacktrace.c:240
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:364 [inline]
 print_report+0x174/0x514 mm/kasan/report.c:475
 kasan_report+0xd8/0x138 mm/kasan/report.c:588
 kasan_check_range+0x254/0x294 mm/kasan/generic.c:187
 __kasan_check_write+0x20/0x30 mm/kasan/shadow.c:37
 instrument_atomic_read_write include/linux/instrumented.h:96 [inline]
 atomic_fetch_add_relaxed include/linux/atomic/atomic-instrumented.h:252 [inline]
 __refcount_add include/linux/refcount.h:193 [inline]
 __refcount_inc include/linux/refcount.h:250 [inline]
 refcount_inc include/linux/refcount.h:267 [inline]
 sock_hold include/net/sock.h:777 [inline]
 sco_sock_timeout+0x64/0x25c net/bluetooth/sco.c:88
 process_one_work+0x694/0x1204 kernel/workqueue.c:2630
 process_scheduled_works kernel/workqueue.c:2703 [inline]
 worker_thread+0x938/0xef4 kernel/workqueue.c:2784
 kthread+0x288/0x310 kernel/kthread.c:388
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:857

Allocated by task 6180:
 kasan_save_stack mm/kasan/common.c:45 [inline]
 kasan_set_track+0x4c/0x7c mm/kasan/common.c:52
 kasan_save_alloc_info+0x24/0x30 mm/kasan/generic.c:511
 ____kasan_kmalloc mm/kasan/common.c:374 [inline]
 __kasan_kmalloc+0xac/0xc4 mm/kasan/common.c:383
 kasan_kmalloc include/linux/kasan.h:198 [inline]
 __do_kmalloc_node mm/slab_common.c:1026 [inline]
 __kmalloc+0xcc/0x1b8 mm/slab_common.c:1039
 kmalloc include/linux/slab.h:603 [inline]
 sk_prot_alloc+0xc4/0x1f0 net/core/sock.c:2090
 sk_alloc+0x44/0x3f4 net/core/sock.c:2143
 bt_sock_alloc+0x4c/0x32c net/bluetooth/af_bluetooth.c:148
 sco_sock_alloc net/bluetooth/sco.c:495 [inline]
 sco_sock_create+0xbc/0x31c net/bluetooth/sco.c:526
 bt_sock_create+0x14c/0x248 net/bluetooth/af_bluetooth.c:132
 __sock_create+0x43c/0x884 net/socket.c:1569
 sock_create net/socket.c:1620 [inline]
 __sys_socket_create net/socket.c:1657 [inline]
 __sys_socket+0x134/0x340 net/socket.c:1708
 __do_sys_socket net/socket.c:1722 [inline]
 __se_sys_socket net/socket.c:1720 [inline]
 __arm64_sys_socket+0x7c/0x94 net/socket.c:1720
 __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
 invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:51
 el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:136
 do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:155
 el0_svc+0x54/0x158 arch/arm64/kernel/entry-common.c:678
 el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:696
 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:595

Freed by task 6179:
 kasan_save_stack mm/kasan/common.c:45 [inline]
 kasan_set_track+0x4c/0x7c mm/kasan/common.c:52
 kasan_save_free_info+0x38/0x5c mm/kasan/generic.c:522
 ____kasan_slab_free+0x144/0x1c0 mm/kasan/common.c:236
 __kasan_slab_free+0x18/0x28 mm/kasan/common.c:244
 kasan_slab_free include/linux/kasan.h:164 [inline]
 slab_free_hook mm/slub.c:1800 [inline]
 slab_free_freelist_hook mm/slub.c:1826 [inline]
 slab_free mm/slub.c:3809 [inline]
 __kmem_cache_free+0x2ac/0x480 mm/slub.c:3822
 kfree+0xb8/0x19c mm/slab_common.c:1075
 sk_prot_free net/core/sock.c:2126 [inline]
 __sk_destruct+0x4c0/0x770 net/core/sock.c:2218
 sk_destruct net/core/sock.c:2233 [inline]
 __sk_free+0x37c/0x4e8 net/core/sock.c:2244
 sk_free+0x60/0xc8 net/core/sock.c:2255
 sock_put include/net/sock.h:1989 [inline]
 sco_sock_kill+0xfc/0x1b4 net/bluetooth/sco.c:426
 sco_sock_release+0x1fc/0x2c0 net/bluetooth/sco.c:1256
 __sock_release net/socket.c:659 [inline]
 sock_close+0xa4/0x1e8 net/socket.c:1419
 __fput+0x324/0x7f8 fs/file_table.c:384
 __fput_sync+0x60/0x9c fs/file_table.c:465
 __do_sys_close fs/open.c:1572 [inline]
 __se_sys_close fs/open.c:1557 [inline]
 __arm64_sys_close+0x150/0x1e0 fs/open.c:1557
 __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
 invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:51
 el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:136
 do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:155
 el0_svc+0x54/0x158 arch/arm64/kernel/entry-common.c:678
 el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:696
 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:595

The buggy address belongs to the object at ffff0000dba59000
 which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 128 bytes inside of
 freed 2048-byte region [ffff0000dba59000, ffff0000dba59800)

The buggy address belongs to the physical page:
page:00000000f24a79df refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x11ba58
head:00000000f24a79df order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
flags: 0x5ffc00000000840(slab|head|node=0|zone=2|lastcpupid=0x7ff)
page_type: 0xffffffff()
raw: 05ffc00000000840 ffff0000c0002000 dead000000000122 0000000000000000
raw: 0000000000000000 0000000000080008 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff0000dba58f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff0000dba59000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff0000dba59080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                   ^
 ffff0000dba59100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff0000dba59180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
------------[ cut here ]------------
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 0 PID: 10 at lib/refcount.c:25 refcount_warn_saturate+0x1a8/0x20c lib/refcount.c:25
Modules linked in:
CPU: 0 PID: 10 Comm: kworker/0:1 Tainted: G    B              6.6.0-rc7-syzkaller-g8de1e7afcc1c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
Workqueue: events sco_sock_timeout
pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : refcount_warn_saturate+0x1a8/0x20c lib/refcount.c:25
lr : refcount_warn_saturate+0x1a8/0x20c lib/refcount.c:25
sp : ffff800092d57af0
x29: ffff800092d57af0 x28: 1fffe0001a9b5a4a x27: dfff800000000000
x26: ffff0000c1084008 x25: ffff0000d4dad250 x24: ffff0001b418b500
x23: dfff800000000000 x22: 0000000000000000 x21: 0000000000000002
x20: ffff0000dba59080 x19: ffff8000910a2000 x18: ffff800092d57800
x17: 0000000000000000 x16: ffff80008a71b23c x15: 0000000000000001
x14: 1ffff000125aaeb0 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000001 x10: 0000000000000000 x9 : c0b0806111008b00
x8 : c0b0806111008b00 x7 : 0000000000000001 x6 : 0000000000000001
x5 : ffff800092d573d8 x4 : ffff80008e4210a0 x3 : ffff800082b180c4
x2 : 0000000000000001 x1 : 0000000000000001 x0 : 0000000000000000
Call trace:
 refcount_warn_saturate+0x1a8/0x20c lib/refcount.c:25
 __refcount_inc include/linux/refcount.h:250 [inline]
 refcount_inc include/linux/refcount.h:267 [inline]
 sock_hold include/net/sock.h:777 [inline]
 sco_sock_timeout+0x19c/0x25c net/bluetooth/sco.c:88
 process_one_work+0x694/0x1204 kernel/workqueue.c:2630
 process_scheduled_works kernel/workqueue.c:2703 [inline]
 worker_thread+0x938/0xef4 kernel/workqueue.c:2784
 kthread+0x288/0x310 kernel/kthread.c:388
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:857
irq event stamp: 29659
hardirqs last  enabled at (29659): [<ffff80008a719090>] __exit_to_kernel_mode arch/arm64/kernel/entry-common.c:84 [inline]
hardirqs last  enabled at (29659): [<ffff80008a719090>] exit_to_kernel_mode+0xdc/0x10c arch/arm64/kernel/entry-common.c:94
hardirqs last disabled at (29658): [<ffff800080021724>] __do_softirq+0x950/0xd54 kernel/softirq.c:569
softirqs last  enabled at (19646): [<ffff800080021894>] softirq_handle_end kernel/softirq.c:399 [inline]
softirqs last  enabled at (19646): [<ffff800080021894>] __do_softirq+0xac0/0xd54 kernel/softirq.c:582
softirqs last disabled at (19597): [<ffff80008002aadc>] ____do_softirq+0x14/0x20 arch/arm64/kernel/irq.c:80
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 10 at lib/refcount.c:28 refcount_warn_saturate+0x1c8/0x20c lib/refcount.c:28
Modules linked in:
CPU: 0 PID: 10 Comm: kworker/0:1 Tainted: G    B   W          6.6.0-rc7-syzkaller-g8de1e7afcc1c #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/06/2023
Workqueue: events sco_sock_timeout
pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : refcount_warn_saturate+0x1c8/0x20c lib/refcount.c:28
lr : refcount_warn_saturate+0x1c8/0x20c lib/refcount.c:28
sp : ffff800092d57af0
x29: ffff800092d57af0 x28: 1fffe0001a9b5a4a x27: dfff800000000000
x26: ffff0000c1084008 x25: ffff0000d4dad250 x24: ffff0001b418b500
x23: dfff800000000000 x22: 0000000000000000 x21: 0000000000000003
x20: ffff0000dba59080 x19: ffff8000910a2000 x18: ffff800092d57800
x17: 0000000000000000 x16: ffff80008a71b23c x15: 0000000000000001
x14: 1fffe0003682f032 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000000 x9 : c0b0806111008b00
x8 : c0b0806111008b00 x7 : 0000000000000001 x6 : 0000000000000001
x5 : ffff800092d573d8 x4 : ffff80008e4210a0 x3 : ffff8000805a359c
x2 : 0000000000000001 x1 : 0000000100000000 x0 : 0000000000000000
Call trace:
 refcount_warn_saturate+0x1c8/0x20c lib/refcount.c:28
 __refcount_sub_and_test include/linux/refcount.h:283 [inline]
 __refcount_dec_and_test include/linux/refcount.h:315 [inline]
 refcount_dec_and_test include/linux/refcount.h:333 [inline]
 sock_put include/net/sock.h:1988 [inline]
 sco_sock_timeout+0x1b0/0x25c net/bluetooth/sco.c:100
 process_one_work+0x694/0x1204 kernel/workqueue.c:2630
 process_scheduled_works kernel/workqueue.c:2703 [inline]
 worker_thread+0x938/0xef4 kernel/workqueue.c:2784
 kthread+0x288/0x310 kernel/kthread.c:388
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:857
irq event stamp: 29659
hardirqs last  enabled at (29659): [<ffff80008a719090>] __exit_to_kernel_mode arch/arm64/kernel/entry-common.c:84 [inline]
hardirqs last  enabled at (29659): [<ffff80008a719090>] exit_to_kernel_mode+0xdc/0x10c arch/arm64/kernel/entry-common.c:94
hardirqs last disabled at (29658): [<ffff800080021724>] __do_softirq+0x950/0xd54 kernel/softirq.c:569
softirqs last  enabled at (19646): [<ffff800080021894>] softirq_handle_end kernel/softirq.c:399 [inline]
softirqs last  enabled at (19646): [<ffff800080021894>] __do_softirq+0xac0/0xd54 kernel/softirq.c:582
softirqs last disabled at (19597): [<ffff80008002aadc>] ____do_softirq+0x14/0x20 arch/arm64/kernel/irq.c:80
---[ end trace 0000000000000000 ]---


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply	[flat|nested] 17+ messages in thread
[parent not found: <000000000000797bd1060a457c08@google.com>]
* Re: [PATCH v1] Bluetooth: SCO: Use disable_delayed_work_sync
@ 2024-10-01 19:49 Luiz Augusto von Dentz
  2024-10-01 20:13 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-01 19:49 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Tue, Oct 1, 2024 at 3:48 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This makes use of disable_delayed_work_sync instead
> cancel_delayed_work_sync as it not only cancel the ongoing work but also
> disables new submit which is disarable since the object holding the work
> is about to be freed.
>
> Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/sco.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index a5ac160c592e..f0604d7834df 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
>         }
>
>         /* Ensure no more work items will run before freeing conn. */
> -       cancel_delayed_work_sync(&conn->timeout_work);
> +       disable_delayed_work_sync(&conn->timeout_work);
>
>         hcon->sco_data = NULL;
>         kfree(conn);
> --
> 2.46.1
>


-- 
Luiz Augusto von Dentz

[-- Attachment #2: v1-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 1180 bytes --]

From a41875885eb7fee3d65479a71e85676699afb820 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v1] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..f0604d7834df 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 	}
 
 	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
+	disable_delayed_work_sync(&conn->timeout_work);
 
 	hcon->sco_data = NULL;
 	kfree(conn);
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v2] Bluetooth: SCO: Use disable_delayed_work_sync
@ 2024-10-02 18:26 Luiz Augusto von Dentz
  2024-10-02 18:46 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-02 18:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Wed, Oct 2, 2024 at 11:40 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This makes use of disable_delayed_work_sync instead
> cancel_delayed_work_sync as it not only cancel the ongoing work but also
> disables new submit which is disarable since the object holding the work
> is about to be freed.
>
> In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> since at that point it is useless to set a timer as the sk will be freed
> there is nothing to be done in sco_sock_timeout.
>
> Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/sco.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index a5ac160c592e..8dfb53dabbd7 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
>         }
>
>         /* Ensure no more work items will run before freeing conn. */
> -       cancel_delayed_work_sync(&conn->timeout_work);
> +       disable_delayed_work_sync(&conn->timeout_work);
>
>         hcon->sco_data = NULL;
>         kfree(conn);
> @@ -444,7 +444,6 @@ static void __sco_sock_close(struct sock *sk)
>         case BT_CONFIG:
>                 if (sco_pi(sk)->conn->hcon) {
>                         sk->sk_state = BT_DISCONN;
> -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
>                         sco_conn_lock(sco_pi(sk)->conn);
>                         hci_conn_drop(sco_pi(sk)->conn->hcon);
>                         sco_pi(sk)->conn->hcon = NULL;
> --
> 2.46.1
>


-- 
Luiz Augusto von Dentz

[-- Attachment #2: v2-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 1764 bytes --]

From a2dbe18d66307323851d19acdae0ed10c6edb72a Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v2] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..8dfb53dabbd7 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 	}
 
 	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
+	disable_delayed_work_sync(&conn->timeout_work);
 
 	hcon->sco_data = NULL;
 	kfree(conn);
@@ -444,7 +444,6 @@ static void __sco_sock_close(struct sock *sk)
 	case BT_CONFIG:
 		if (sco_pi(sk)->conn->hcon) {
 			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
 			sco_conn_lock(sco_pi(sk)->conn);
 			hci_conn_drop(sco_pi(sk)->conn->hcon);
 			sco_pi(sk)->conn->hcon = NULL;
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
@ 2024-10-02 19:19 Luiz Augusto von Dentz
  2024-10-02 19:37 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-02 19:19 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

#syz test

On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This makes use of disable_delayed_work_sync instead
> cancel_delayed_work_sync as it not only cancel the ongoing work but also
> disables new submit which is disarable since the object holding the work
> is about to be freed.
>
> In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> since at that point it is useless to set a timer as the sk will be freed
> there is nothing to be done in sco_sock_timeout.
>
> Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  net/bluetooth/sco.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index a5ac160c592e..2b1e66976068 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
>         }
>
>         /* Ensure no more work items will run before freeing conn. */
> -       cancel_delayed_work_sync(&conn->timeout_work);
> +       disable_delayed_work_sync(&conn->timeout_work);
>
>         hcon->sco_data = NULL;
>         kfree(conn);
> @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
>
>         case BT_CONNECTED:
>         case BT_CONFIG:
> -               if (sco_pi(sk)->conn->hcon) {
> -                       sk->sk_state = BT_DISCONN;
> -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> -                       sco_conn_lock(sco_pi(sk)->conn);
> -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> -                       sco_pi(sk)->conn->hcon = NULL;
> -                       sco_conn_unlock(sco_pi(sk)->conn);
> -               } else
> -                       sco_chan_del(sk, ECONNRESET);
> -               break;
> -
>         case BT_CONNECT2:
>         case BT_CONNECT:
>         case BT_DISCONN:
> --
> 2.46.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-02 19:19 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-02 19:46 Luiz Augusto von Dentz
  2024-10-02 20:05 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-02 19:46 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This makes use of disable_delayed_work_sync instead
> > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > disables new submit which is disarable since the object holding the work
> > is about to be freed.
> >
> > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > since at that point it is useless to set a timer as the sk will be freed
> > there is nothing to be done in sco_sock_timeout.
> >
> > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> >  net/bluetooth/sco.c | 13 +------------
> >  1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index a5ac160c592e..2b1e66976068 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> >         }
> >
> >         /* Ensure no more work items will run before freeing conn. */
> > -       cancel_delayed_work_sync(&conn->timeout_work);
> > +       disable_delayed_work_sync(&conn->timeout_work);
> >
> >         hcon->sco_data = NULL;
> >         kfree(conn);
> > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> >
> >         case BT_CONNECTED:
> >         case BT_CONFIG:
> > -               if (sco_pi(sk)->conn->hcon) {
> > -                       sk->sk_state = BT_DISCONN;
> > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > -                       sco_conn_lock(sco_pi(sk)->conn);
> > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > -                       sco_pi(sk)->conn->hcon = NULL;
> > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > -               } else
> > -                       sco_chan_del(sk, ECONNRESET);
> > -               break;
> > -
> >         case BT_CONNECT2:
> >         case BT_CONNECT:
> >         case BT_DISCONN:
> > --
> > 2.46.1
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v3-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 1953 bytes --]

From 0d8909030c2d82967efc93a008b92610a3d77b0d Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..2b1e66976068 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 	}
 
 	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
+	disable_delayed_work_sync(&conn->timeout_work);
 
 	hcon->sco_data = NULL;
 	kfree(conn);
@@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-02 19:46 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-02 20:46 Luiz Augusto von Dentz
  2024-10-02 23:16 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-02 20:46 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This makes use of disable_delayed_work_sync instead
> > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > disables new submit which is disarable since the object holding the work
> > > is about to be freed.
> > >
> > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > since at that point it is useless to set a timer as the sk will be freed
> > > there is nothing to be done in sco_sock_timeout.
> > >
> > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > ---
> > >  net/bluetooth/sco.c | 13 +------------
> > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > >
> > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > index a5ac160c592e..2b1e66976068 100644
> > > --- a/net/bluetooth/sco.c
> > > +++ b/net/bluetooth/sco.c
> > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > >         }
> > >
> > >         /* Ensure no more work items will run before freeing conn. */
> > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > +       disable_delayed_work_sync(&conn->timeout_work);
> > >
> > >         hcon->sco_data = NULL;
> > >         kfree(conn);
> > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > >
> > >         case BT_CONNECTED:
> > >         case BT_CONFIG:
> > > -               if (sco_pi(sk)->conn->hcon) {
> > > -                       sk->sk_state = BT_DISCONN;
> > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > -               } else
> > > -                       sco_chan_del(sk, ECONNRESET);
> > > -               break;
> > > -
> > >         case BT_CONNECT2:
> > >         case BT_CONNECT:
> > >         case BT_DISCONN:
> > > --
> > > 2.46.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 2293 bytes --]

From 85b438673dd4f1bb68676294d5674d20a0d47c09 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..cee87c6c9194 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -169,10 +169,11 @@ static void sco_chan_del(struct sock *sk, int err)
 		sco_conn_lock(conn);
 		conn->sk = NULL;
 		sco_pi(sk)->conn = NULL;
-		sco_conn_unlock(conn);
-
-		if (conn->hcon)
+		if (conn->hcon) {
 			hci_conn_drop(conn->hcon);
+			conn->hcon = NULL;
+		}
+		sco_conn_unlock(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -208,7 +209,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 	}
 
 	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
+	disable_delayed_work_sync(&conn->timeout_work);
 
 	hcon->sco_data = NULL;
 	kfree(conn);
@@ -442,17 +443,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-02 20:46 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-03 15:38 Luiz Augusto von Dentz
  2024-10-03 15:55 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-03 15:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This makes use of disable_delayed_work_sync instead
> > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > disables new submit which is disarable since the object holding the work
> > > > is about to be freed.
> > > >
> > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > since at that point it is useless to set a timer as the sk will be freed
> > > > there is nothing to be done in sco_sock_timeout.
> > > >
> > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > ---
> > > >  net/bluetooth/sco.c | 13 +------------
> > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > index a5ac160c592e..2b1e66976068 100644
> > > > --- a/net/bluetooth/sco.c
> > > > +++ b/net/bluetooth/sco.c
> > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > >         }
> > > >
> > > >         /* Ensure no more work items will run before freeing conn. */
> > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > >
> > > >         hcon->sco_data = NULL;
> > > >         kfree(conn);
> > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > >
> > > >         case BT_CONNECTED:
> > > >         case BT_CONFIG:
> > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > -                       sk->sk_state = BT_DISCONN;
> > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > -               } else
> > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > -               break;
> > > > -
> > > >         case BT_CONNECT2:
> > > >         case BT_CONNECT:
> > > >         case BT_DISCONN:
> > > > --
> > > > 2.46.1
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 3708 bytes --]

From 600534815b145102456b0f2eada005bd3e89ea6b Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 66 ++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..6aa08e709391 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -155,6 +155,28 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void sco_conn_destruct(struct sco_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	if (conn->hcon) {
+		hci_conn_drop(conn->hcon);
+
+		sco_conn_lock(conn);
+		conn->hcon->sco_data = NULL;
+		conn->hcon = NULL;
+		sco_conn_unlock(conn);
+	}
+
+	/* Ensure no more work items will run before freeing conn. */
+	disable_delayed_work_sync(&conn->timeout_work);
+
+	kfree(conn);
+}
+
 /* Delete channel.
  * Must be called on the locked socket. */
 static void sco_chan_del(struct sock *sk, int err)
@@ -165,16 +187,6 @@ static void sco_chan_del(struct sock *sk, int err)
 
 	BT_DBG("sk %p, conn %p, err %d", sk, conn, err);
 
-	if (conn) {
-		sco_conn_lock(conn);
-		conn->sk = NULL;
-		sco_pi(sk)->conn = NULL;
-		sco_conn_unlock(conn);
-
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
-	}
-
 	sk->sk_state = BT_CLOSED;
 	sk->sk_err   = err;
 	sk->sk_state_change(sk);
@@ -192,26 +204,23 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
-	/* Kill socket */
 	sco_conn_lock(conn);
 	sk = conn->sk;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		sco_sock_clear_timer(sk);
-		sco_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		sco_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->sco_data = NULL;
-	kfree(conn);
+	/* Kill socket */
+	lock_sock(sk);
+	sco_sock_clear_timer(sk);
+	sco_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
@@ -395,6 +404,8 @@ static void sco_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	sco_conn_destruct(sco_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -442,17 +453,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-03 15:38 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-03 16:32 Luiz Augusto von Dentz
  2024-10-03 16:53 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-03 16:32 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > >
> > > > > This makes use of disable_delayed_work_sync instead
> > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > disables new submit which is disarable since the object holding the work
> > > > > is about to be freed.
> > > > >
> > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > there is nothing to be done in sco_sock_timeout.
> > > > >
> > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > ---
> > > > >  net/bluetooth/sco.c | 13 +------------
> > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > --- a/net/bluetooth/sco.c
> > > > > +++ b/net/bluetooth/sco.c
> > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > >         }
> > > > >
> > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > >
> > > > >         hcon->sco_data = NULL;
> > > > >         kfree(conn);
> > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > >
> > > > >         case BT_CONNECTED:
> > > > >         case BT_CONFIG:
> > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > -               } else
> > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > -               break;
> > > > > -
> > > > >         case BT_CONNECT2:
> > > > >         case BT_CONNECT:
> > > > >         case BT_DISCONN:
> > > > > --
> > > > > 2.46.1
> > > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 3733 bytes --]

From 369c62c6d6f243177ce0b1261c6eeda6ff81d097 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 66 +++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..52f7382b1fcc 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -155,6 +155,26 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void sco_conn_destruct(struct sco_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	if (conn->hcon) {
+		sco_conn_lock(conn);
+		conn->hcon->sco_data = NULL;
+		conn->hcon = NULL;
+		sco_conn_unlock(conn);
+	}
+
+	/* Ensure no more work items will run before freeing conn. */
+	disable_delayed_work_sync(&conn->timeout_work);
+
+	kfree(conn);
+}
+
 /* Delete channel.
  * Must be called on the locked socket. */
 static void sco_chan_del(struct sock *sk, int err)
@@ -165,15 +185,9 @@ static void sco_chan_del(struct sock *sk, int err)
 
 	BT_DBG("sk %p, conn %p, err %d", sk, conn, err);
 
-	if (conn) {
-		sco_conn_lock(conn);
-		conn->sk = NULL;
-		sco_pi(sk)->conn = NULL;
-		sco_conn_unlock(conn);
-
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
-	}
+	/* Drop HCI connection */
+	if (conn && conn->hcon)
+		hci_conn_drop(conn->hcon);
 
 	sk->sk_state = BT_CLOSED;
 	sk->sk_err   = err;
@@ -192,26 +206,23 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
-	/* Kill socket */
 	sco_conn_lock(conn);
 	sk = conn->sk;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		sco_sock_clear_timer(sk);
-		sco_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		sco_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->sco_data = NULL;
-	kfree(conn);
+	/* Kill socket */
+	lock_sock(sk);
+	sco_sock_clear_timer(sk);
+	sco_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
@@ -395,6 +406,8 @@ static void sco_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	sco_conn_destruct(sco_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -442,17 +455,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-03 16:32 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-03 19:21 Luiz Augusto von Dentz
  2024-10-03 19:44 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-03 19:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > >
> > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > disables new submit which is disarable since the object holding the work
> > > > > > is about to be freed.
> > > > > >
> > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > >
> > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > ---
> > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > >
> > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > --- a/net/bluetooth/sco.c
> > > > > > +++ b/net/bluetooth/sco.c
> > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > >         }
> > > > > >
> > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > >
> > > > > >         hcon->sco_data = NULL;
> > > > > >         kfree(conn);
> > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > >
> > > > > >         case BT_CONNECTED:
> > > > > >         case BT_CONFIG:
> > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > -               } else
> > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > -               break;
> > > > > > -
> > > > > >         case BT_CONNECT2:
> > > > > >         case BT_CONNECT:
> > > > > >         case BT_DISCONN:
> > > > > > --
> > > > > > 2.46.1
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0002-Bluetooth-ISO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 3405 bytes --]

From 87b1ba6a0644ada293d1687eb382d94ae8269f3b Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 16:15:51 -0400
Subject: [PATCH v4 2/2] Bluetooth: ISO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancels the ongoing work but also
disables new submissions which is disarable since the object holding the
work is about to be freed.

In addition to it remove call to iso_sock_set_timer on iso_sock_disconn
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in iso_sock_timeout.

Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/iso.c | 55 +++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index d5e00d0dd1a0..cff04efc59f7 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -163,6 +163,21 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void iso_conn_drop(struct iso_conn *conn)
+{
+	if (!conn || !conn->hcon)
+		return;
+
+	BT_DBG("conn %p hcon %p", conn, conn->hcon);
+
+	hci_conn_drop(conn->hcon);
+
+	iso_conn_lock(conn);
+	conn->hcon->iso_data = NULL;
+	conn->hcon = NULL;
+	iso_conn_unlock(conn);
+}
+
 /* Delete channel. Must be called on the locked socket. */
 static void iso_chan_del(struct sock *sk, int err)
 {
@@ -179,8 +194,7 @@ static void iso_chan_del(struct sock *sk, int err)
 		iso_pi(sk)->conn = NULL;
 		iso_conn_unlock(conn);
 
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
+		iso_conn_drop(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -197,6 +211,21 @@ static void iso_chan_del(struct sock *sk, int err)
 	sock_set_flag(sk, SOCK_ZAPPED);
 }
 
+static void iso_conn_destruct(struct iso_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	iso_conn_drop(conn);
+
+	/* Ensure no more work items will run before freeing conn. */
+	disable_delayed_work_sync(&conn->timeout_work);
+
+	kfree(conn);
+}
+
 static void iso_conn_del(struct hci_conn *hcon, int err)
 {
 	struct iso_conn *conn = hcon->iso_data;
@@ -214,19 +243,16 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
 		sock_hold(sk);
 	iso_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		iso_sock_clear_timer(sk);
-		iso_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		iso_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->iso_data = NULL;
-	kfree(conn);
+	lock_sock(sk);
+	iso_sock_clear_timer(sk);
+	iso_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static int __iso_chan_add(struct iso_conn *conn, struct sock *sk,
@@ -646,6 +672,8 @@ static void iso_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	iso_conn_destruct(iso_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -714,7 +742,6 @@ static void iso_sock_disconn(struct sock *sk)
 	}
 
 	sk->sk_state = BT_DISCONN;
-	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
 	iso_conn_lock(iso_pi(sk)->conn);
 	hci_conn_drop(iso_pi(sk)->conn->hcon);
 	iso_pi(sk)->conn->hcon = NULL;
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-03 19:21 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-04 16:06 Luiz Augusto von Dentz
  2024-10-04 16:34 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-04 16:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > >
> > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > is about to be freed.
> > > > > > >
> > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > >
> > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > ---
> > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > >
> > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > >         }
> > > > > > >
> > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > >
> > > > > > >         hcon->sco_data = NULL;
> > > > > > >         kfree(conn);
> > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > >
> > > > > > >         case BT_CONNECTED:
> > > > > > >         case BT_CONFIG:
> > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > -               } else
> > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > -               break;
> > > > > > > -
> > > > > > >         case BT_CONNECT2:
> > > > > > >         case BT_CONNECT:
> > > > > > >         case BT_DISCONN:
> > > > > > > --
> > > > > > > 2.46.1
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 3726 bytes --]

From 4122dbcb847ebbc4ae72620230bbc8b926cd44f5 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4 1/2] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 67 ++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 25 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..19af148bbaf8 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -155,6 +155,36 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void sco_conn_drop(struct sco_conn *conn)
+{
+	if (!conn || !conn->hcon)
+		return;
+
+	BT_DBG("conn %p hcon %p", conn, conn->hcon);
+
+	hci_conn_drop(conn->hcon);
+
+	sco_conn_lock(conn);
+	conn->hcon->sco_data = NULL;
+	conn->hcon = NULL;
+	sco_conn_unlock(conn);
+
+	/* Ensure no more work items will run since hci_conn has been dropped */
+	disable_delayed_work_sync(&conn->timeout_work);
+}
+
+static void sco_conn_destruct(struct sco_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	sco_conn_drop(conn);
+
+	kfree(conn);
+}
+
 /* Delete channel.
  * Must be called on the locked socket. */
 static void sco_chan_del(struct sock *sk, int err)
@@ -171,8 +201,7 @@ static void sco_chan_del(struct sock *sk, int err)
 		sco_pi(sk)->conn = NULL;
 		sco_conn_unlock(conn);
 
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
+		sco_conn_drop(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -192,26 +221,23 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
-	/* Kill socket */
 	sco_conn_lock(conn);
 	sk = conn->sk;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		sco_sock_clear_timer(sk);
-		sco_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		sco_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->sco_data = NULL;
-	kfree(conn);
+	/* Kill socket */
+	lock_sock(sk);
+	sco_sock_clear_timer(sk);
+	sco_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
@@ -395,6 +421,8 @@ static void sco_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	sco_conn_destruct(sco_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -442,17 +470,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-04 16:06 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-04 17:24 Luiz Augusto von Dentz
  2024-10-04 17:40 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-04 17:24 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > >
> > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > is about to be freed.
> > > > > > > >
> > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > >
> > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > ---
> > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > >         }
> > > > > > > >
> > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > >
> > > > > > > >         hcon->sco_data = NULL;
> > > > > > > >         kfree(conn);
> > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > >
> > > > > > > >         case BT_CONNECTED:
> > > > > > > >         case BT_CONFIG:
> > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > -               } else
> > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > -               break;
> > > > > > > > -
> > > > > > > >         case BT_CONNECT2:
> > > > > > > >         case BT_CONNECT:
> > > > > > > >         case BT_DISCONN:
> > > > > > > > --
> > > > > > > > 2.46.1
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0002-Bluetooth-ISO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 3468 bytes --]

From 0534aba3c6e0f8bebb003516e433dcbddd3e2014 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 16:15:51 -0400
Subject: [PATCH v4 2/2] Bluetooth: ISO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancels the ongoing work but also
disables new submissions which is disarable since the object holding the
work is about to be freed.

In addition to it remove call to iso_sock_set_timer on iso_sock_disconn
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in iso_sock_timeout.

Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/iso.c | 58 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index d5e00d0dd1a0..030d402cc9bd 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -163,6 +163,24 @@ static struct iso_conn *iso_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void iso_conn_drop(struct iso_conn *conn)
+{
+	if (!conn || !conn->hcon)
+		return;
+
+	BT_DBG("conn %p hcon %p", conn, conn->hcon);
+
+	hci_conn_drop(conn->hcon);
+
+	iso_conn_lock(conn);
+	conn->hcon->iso_data = NULL;
+	conn->hcon = NULL;
+	iso_conn_unlock(conn);
+
+	/* Ensure no more work items will run since hci_conn has been dropped */
+	disable_delayed_work_sync(&conn->timeout_work);
+}
+
 /* Delete channel. Must be called on the locked socket. */
 static void iso_chan_del(struct sock *sk, int err)
 {
@@ -179,8 +197,7 @@ static void iso_chan_del(struct sock *sk, int err)
 		iso_pi(sk)->conn = NULL;
 		iso_conn_unlock(conn);
 
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
+		iso_conn_drop(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -197,6 +214,21 @@ static void iso_chan_del(struct sock *sk, int err)
 	sock_set_flag(sk, SOCK_ZAPPED);
 }
 
+static void iso_conn_destruct(struct iso_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	if (conn->sk)
+		iso_pi(conn->sk)->conn = NULL;
+
+	iso_conn_drop(conn);
+
+	kfree(conn);
+}
+
 static void iso_conn_del(struct hci_conn *hcon, int err)
 {
 	struct iso_conn *conn = hcon->iso_data;
@@ -214,19 +246,16 @@ static void iso_conn_del(struct hci_conn *hcon, int err)
 		sock_hold(sk);
 	iso_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		iso_sock_clear_timer(sk);
-		iso_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		iso_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->iso_data = NULL;
-	kfree(conn);
+	lock_sock(sk);
+	iso_sock_clear_timer(sk);
+	iso_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static int __iso_chan_add(struct iso_conn *conn, struct sock *sk,
@@ -646,6 +675,8 @@ static void iso_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	iso_conn_destruct(iso_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -714,7 +745,6 @@ static void iso_sock_disconn(struct sock *sk)
 	}
 
 	sk->sk_state = BT_DISCONN;
-	iso_sock_set_timer(sk, ISO_DISCONN_TIMEOUT);
 	iso_conn_lock(iso_pi(sk)->conn);
 	hci_conn_drop(iso_pi(sk)->conn->hcon);
 	iso_pi(sk)->conn->hcon = NULL;
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-04 17:24 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-07 17:16 Luiz Augusto von Dentz
  2024-10-07 17:33 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-07 17:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Fri, Oct 4, 2024 at 1:24 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > #syz test
> > > > > > > >
> > > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > >
> > > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > > is about to be freed.
> > > > > > > > >
> > > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > > >
> > > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > ---
> > > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > > >
> > > > > > > > >         hcon->sco_data = NULL;
> > > > > > > > >         kfree(conn);
> > > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > > >
> > > > > > > > >         case BT_CONNECTED:
> > > > > > > > >         case BT_CONFIG:
> > > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > > -               } else
> > > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > > -               break;
> > > > > > > > > -
> > > > > > > > >         case BT_CONNECT2:
> > > > > > > > >         case BT_CONNECT:
> > > > > > > > >         case BT_DISCONN:
> > > > > > > > > --
> > > > > > > > > 2.46.1
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Luiz Augusto von Dentz
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 4031 bytes --]

From af236b58a3bcb25aa15d1fcc977fdbe9ad265cdf Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4 1/2] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 85 ++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 32 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..fab68cb60371 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -155,6 +155,46 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	return conn;
 }
 
+static void sco_conn_drop(struct sco_conn *conn)
+{
+	if (!conn || (!conn->hcon && !conn->sk))
+		return;
+
+	BT_DBG("conn %p hcon %p", conn, conn->hcon);
+
+	sco_conn_lock(conn);
+
+	if (conn->sk) {
+		sco_pi(conn->sk)->conn = NULL;
+		conn->sk = NULL;
+	}
+
+	if (conn->hcon) {
+		struct hci_conn *hcon = conn->hcon;
+
+		conn->hcon->sco_data = NULL;
+		conn->hcon = NULL;
+		hci_conn_drop(hcon);
+	}
+
+	sco_conn_unlock(conn);
+
+	/* Ensure no more work items will run since hci_conn has been dropped */
+	disable_delayed_work_sync(&conn->timeout_work);
+}
+
+static void sco_conn_destruct(struct sco_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p", conn);
+
+	sco_conn_drop(conn);
+
+	kfree(conn);
+}
+
 /* Delete channel.
  * Must be called on the locked socket. */
 static void sco_chan_del(struct sock *sk, int err)
@@ -165,15 +205,8 @@ static void sco_chan_del(struct sock *sk, int err)
 
 	BT_DBG("sk %p, conn %p, err %d", sk, conn, err);
 
-	if (conn) {
-		sco_conn_lock(conn);
-		conn->sk = NULL;
-		sco_pi(sk)->conn = NULL;
-		sco_conn_unlock(conn);
-
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
-	}
+	if (conn)
+		sco_conn_drop(conn);
 
 	sk->sk_state = BT_CLOSED;
 	sk->sk_err   = err;
@@ -192,26 +225,23 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
-	/* Kill socket */
 	sco_conn_lock(conn);
 	sk = conn->sk;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		sco_sock_clear_timer(sk);
-		sco_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		sco_conn_destruct(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->sco_data = NULL;
-	kfree(conn);
+	/* Kill socket */
+	lock_sock(sk);
+	sco_sock_clear_timer(sk);
+	sco_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
@@ -395,6 +425,8 @@ static void sco_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	sco_conn_destruct(sco_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -442,17 +474,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-07 17:16 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-07 20:54 Luiz Augusto von Dentz
  2024-10-07 21:15 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-07 20:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Mon, Oct 7, 2024 at 1:16 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Fri, Oct 4, 2024 at 1:24 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > #syz test
> > > > > > > >
> > > > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > #syz test
> > > > > > > > >
> > > > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > >
> > > > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > > > is about to be freed.
> > > > > > > > > >
> > > > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > > > >
> > > > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > ---
> > > > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > >
> > > > > > > > > >         hcon->sco_data = NULL;
> > > > > > > > > >         kfree(conn);
> > > > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > > > >
> > > > > > > > > >         case BT_CONNECTED:
> > > > > > > > > >         case BT_CONFIG:
> > > > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > > > -               } else
> > > > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > > > -               break;
> > > > > > > > > > -
> > > > > > > > > >         case BT_CONNECT2:
> > > > > > > > > >         case BT_CONNECT:
> > > > > > > > > >         case BT_DISCONN:
> > > > > > > > > > --
> > > > > > > > > > 2.46.1
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Luiz Augusto von Dentz
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Luiz Augusto von Dentz
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v4-0001-Bluetooth-SCO-Use-disable_delayed_work_sync.patch --]
[-- Type: text/x-patch, Size: 5495 bytes --]

From 1202ae4e16c042a149725e30b3e8857120f0f2a7 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 1 Oct 2024 15:46:10 -0400
Subject: [PATCH v4 1/2] Bluetooth: SCO: Use disable_delayed_work_sync

This makes use of disable_delayed_work_sync instead
cancel_delayed_work_sync as it not only cancel the ongoing work but also
disables new submit which is disarable since the object holding the work
is about to be freed.

In addition to it remove call to sco_sock_set_timer on __sco_sock_close
since at that point it is useless to set a timer as the sk will be freed
there is nothing to be done in sco_sock_timeout.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 99 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 71 insertions(+), 28 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..a937c3b9d639 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -51,6 +51,7 @@ struct sco_conn {
 	struct delayed_work	timeout_work;
 
 	unsigned int    mtu;
+	struct kref	ref;
 };
 
 #define sco_conn_lock(c)	spin_lock(&c->lock)
@@ -76,12 +77,59 @@ struct sco_pinfo {
 #define SCO_CONN_TIMEOUT	(HZ * 40)
 #define SCO_DISCONN_TIMEOUT	(HZ * 2)
 
+static void sco_conn_free(struct kref *ref)
+{
+	struct sco_conn *conn = container_of(ref, struct sco_conn, ref);
+
+	BT_DBG("conn %p", conn);
+
+	if (conn->sk)
+		sco_pi(conn->sk)->conn = NULL;
+
+	if (conn->hcon) {
+		conn->hcon->sco_data = NULL;
+		hci_conn_drop(conn->hcon);
+	}
+
+	/* Ensure no more work items will run since hci_conn has been dropped */
+	disable_delayed_work_sync(&conn->timeout_work);
+
+	kfree(conn);
+}
+
+static void sco_conn_put(struct sco_conn *conn)
+{
+	if (!conn)
+		return;
+
+	BT_DBG("conn %p refcnt %d", conn, kref_read(&conn->ref));
+
+	kref_put(&conn->ref, sco_conn_free);
+}
+
+static struct sco_conn *sco_conn_hold_unless_zero(struct sco_conn *conn)
+{
+	if (!conn)
+		return NULL;
+
+	BT_DBG("conn %p refcnt %u", conn, kref_read(&conn->ref));
+
+	if (!kref_get_unless_zero(&conn->ref))
+		return NULL;
+
+	return conn;
+}
+
 static void sco_sock_timeout(struct work_struct *work)
 {
 	struct sco_conn *conn = container_of(work, struct sco_conn,
 					     timeout_work.work);
 	struct sock *sk;
 
+	conn = sco_conn_hold_unless_zero(conn);
+	if (!conn)
+		return;
+
 	sco_conn_lock(conn);
 	if (!conn->hcon) {
 		sco_conn_unlock(conn);
@@ -91,6 +139,7 @@ static void sco_sock_timeout(struct work_struct *work)
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
+	sco_conn_put(conn);
 
 	if (!sk)
 		return;
@@ -128,9 +177,14 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 {
 	struct sco_conn *conn = hcon->sco_data;
 
+	conn = sco_conn_hold_unless_zero(conn);
 	if (conn) {
-		if (!conn->hcon)
+		if (!conn->hcon) {
+			sco_conn_lock(conn);
 			conn->hcon = hcon;
+			sco_conn_unlock(conn);
+		}
+		sco_conn_put(conn);
 		return conn;
 	}
 
@@ -138,6 +192,7 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon)
 	if (!conn)
 		return NULL;
 
+	kref_init(&conn->ref);
 	spin_lock_init(&conn->lock);
 	INIT_DELAYED_WORK(&conn->timeout_work, sco_sock_timeout);
 
@@ -162,17 +217,15 @@ static void sco_chan_del(struct sock *sk, int err)
 	struct sco_conn *conn;
 
 	conn = sco_pi(sk)->conn;
+	sco_pi(sk)->conn = NULL;
 
 	BT_DBG("sk %p, conn %p, err %d", sk, conn, err);
 
 	if (conn) {
 		sco_conn_lock(conn);
 		conn->sk = NULL;
-		sco_pi(sk)->conn = NULL;
 		sco_conn_unlock(conn);
-
-		if (conn->hcon)
-			hci_conn_drop(conn->hcon);
+		sco_conn_put(conn);
 	}
 
 	sk->sk_state = BT_CLOSED;
@@ -187,31 +240,30 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 	struct sco_conn *conn = hcon->sco_data;
 	struct sock *sk;
 
+	conn = sco_conn_hold_unless_zero(conn);
 	if (!conn)
 		return;
 
 	BT_DBG("hcon %p conn %p, err %d", hcon, conn, err);
 
-	/* Kill socket */
 	sco_conn_lock(conn);
 	sk = conn->sk;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
+	sco_conn_put(conn);
 
-	if (sk) {
-		lock_sock(sk);
-		sco_sock_clear_timer(sk);
-		sco_chan_del(sk, err);
-		release_sock(sk);
-		sock_put(sk);
+	if (!sk) {
+		sco_conn_put(conn);
+		return;
 	}
 
-	/* Ensure no more work items will run before freeing conn. */
-	cancel_delayed_work_sync(&conn->timeout_work);
-
-	hcon->sco_data = NULL;
-	kfree(conn);
+	/* Kill socket */
+	lock_sock(sk);
+	sco_sock_clear_timer(sk);
+	sco_chan_del(sk, err);
+	release_sock(sk);
+	sock_put(sk);
 }
 
 static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
@@ -395,6 +447,8 @@ static void sco_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
+	sco_conn_put(sco_pi(sk)->conn);
+
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 }
@@ -442,17 +496,6 @@ static void __sco_sock_close(struct sock *sk)
 
 	case BT_CONNECTED:
 	case BT_CONFIG:
-		if (sco_pi(sk)->conn->hcon) {
-			sk->sk_state = BT_DISCONN;
-			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
-			sco_conn_lock(sco_pi(sk)->conn);
-			hci_conn_drop(sco_pi(sk)->conn->hcon);
-			sco_pi(sk)->conn->hcon = NULL;
-			sco_conn_unlock(sco_pi(sk)->conn);
-		} else
-			sco_chan_del(sk, ECONNRESET);
-		break;
-
 	case BT_CONNECT2:
 	case BT_CONNECT:
 	case BT_DISCONN:
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-07 20:54 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-22 16:44 Luiz Augusto von Dentz
  2024-10-22 17:15 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-22 16:44 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Mon, Oct 7, 2024 at 4:54 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Mon, Oct 7, 2024 at 1:16 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Fri, Oct 4, 2024 at 1:24 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > #syz test
> > > > > > > >
> > > > > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > #syz test
> > > > > > > > >
> > > > > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > #syz test
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > >
> > > > > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > > > > is about to be freed.
> > > > > > > > > > >
> > > > > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > > > > >
> > > > > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > > > > >         }
> > > > > > > > > > >
> > > > > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > > >
> > > > > > > > > > >         hcon->sco_data = NULL;
> > > > > > > > > > >         kfree(conn);
> > > > > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > > > > >
> > > > > > > > > > >         case BT_CONNECTED:
> > > > > > > > > > >         case BT_CONFIG:
> > > > > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > > > > -               } else
> > > > > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > > > > -               break;
> > > > > > > > > > > -
> > > > > > > > > > >         case BT_CONNECT2:
> > > > > > > > > > >         case BT_CONNECT:
> > > > > > > > > > >         case BT_DISCONN:
> > > > > > > > > > > --
> > > > > > > > > > > 2.46.1
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Luiz Augusto von Dentz
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Luiz Augusto von Dentz
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Luiz Augusto von Dentz
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v1-0001-Bluetooth-SCO-Fix-UAF-on-sco_sock_timeout.patch --]
[-- Type: text/x-patch, Size: 1735 bytes --]

From 4a960d62b95deab698c4e13af036a3f0589add70 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 22 Oct 2024 12:31:08 -0400
Subject: [PATCH v1] Bluetooth: SCO: Fix UAF on sco_sock_timeout

conn->sk maybe have been unlinked/freed while waiting for sco_conn_lock
so this checks if the conn->sk is still valid by checking if it part of
sco_sk_list.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/sco.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..9a28b2f83e7c 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -76,6 +76,27 @@ struct sco_pinfo {
 #define SCO_CONN_TIMEOUT	(HZ * 40)
 #define SCO_DISCONN_TIMEOUT	(HZ * 2)
 
+static bool sco_conn_linked(struct sco_conn *conn)
+{
+	struct sock *sk;
+
+	if (!conn || !conn->sk)
+		return false;
+
+	read_lock(&sco_sk_list.lock);
+
+	sk_for_each(sk, &sco_sk_list.head) {
+		if (sk == conn->sk) {
+			read_unlock(&sco_sk_list.lock);
+			return true;
+		}
+	}
+
+	read_unlock(&sco_sk_list.lock);
+
+	return false;
+}
+
 static void sco_sock_timeout(struct work_struct *work)
 {
 	struct sco_conn *conn = container_of(work, struct sco_conn,
@@ -87,7 +108,7 @@ static void sco_sock_timeout(struct work_struct *work)
 		sco_conn_unlock(conn);
 		return;
 	}
-	sk = conn->sk;
+	sk = sco_conn_linked(conn) ? conn->sk : NULL;
 	if (sk)
 		sock_hold(sk);
 	sco_conn_unlock(conn);
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync
  2024-10-22 16:44 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
@ 2024-10-22 19:19 Luiz Augusto von Dentz
  2024-10-22 19:51 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
  0 siblings, 1 reply; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-22 19:19 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: syzbot+4c0d0c4cde787116d465

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

#syz test

On Tue, Oct 22, 2024 at 12:44 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> #syz test
>
> On Mon, Oct 7, 2024 at 4:54 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > #syz test
> >
> > On Mon, Oct 7, 2024 at 1:16 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > #syz test
> > >
> > > On Fri, Oct 4, 2024 at 1:24 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > #syz test
> > > >
> > > > On Fri, Oct 4, 2024 at 12:06 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > #syz test
> > > > >
> > > > > On Thu, Oct 3, 2024 at 3:21 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > #syz test
> > > > > >
> > > > > > On Thu, Oct 3, 2024 at 12:32 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > #syz test
> > > > > > >
> > > > > > > On Thu, Oct 3, 2024 at 11:38 AM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > #syz test
> > > > > > > >
> > > > > > > > On Wed, Oct 2, 2024 at 4:46 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > #syz test
> > > > > > > > >
> > > > > > > > > On Wed, Oct 2, 2024 at 3:46 PM Luiz Augusto von Dentz
> > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > #syz test
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 2, 2024 at 3:19 PM Luiz Augusto von Dentz
> > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > #syz test
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Oct 2, 2024 at 3:04 PM Luiz Augusto von Dentz
> > > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > > >
> > > > > > > > > > > > This makes use of disable_delayed_work_sync instead
> > > > > > > > > > > > cancel_delayed_work_sync as it not only cancel the ongoing work but also
> > > > > > > > > > > > disables new submit which is disarable since the object holding the work
> > > > > > > > > > > > is about to be freed.
> > > > > > > > > > > >
> > > > > > > > > > > > In addition to it remove call to sco_sock_set_timer on __sco_sock_close
> > > > > > > > > > > > since at that point it is useless to set a timer as the sk will be freed
> > > > > > > > > > > > there is nothing to be done in sco_sock_timeout.
> > > > > > > > > > > >
> > > > > > > > > > > > Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
> > > > > > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
> > > > > > > > > > > > Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
> > > > > > > > > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >  net/bluetooth/sco.c | 13 +------------
> > > > > > > > > > > >  1 file changed, 1 insertion(+), 12 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > > > > > > > > index a5ac160c592e..2b1e66976068 100644
> > > > > > > > > > > > --- a/net/bluetooth/sco.c
> > > > > > > > > > > > +++ b/net/bluetooth/sco.c
> > > > > > > > > > > > @@ -208,7 +208,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
> > > > > > > > > > > >         }
> > > > > > > > > > > >
> > > > > > > > > > > >         /* Ensure no more work items will run before freeing conn. */
> > > > > > > > > > > > -       cancel_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > > > > +       disable_delayed_work_sync(&conn->timeout_work);
> > > > > > > > > > > >
> > > > > > > > > > > >         hcon->sco_data = NULL;
> > > > > > > > > > > >         kfree(conn);
> > > > > > > > > > > > @@ -442,17 +442,6 @@ static void __sco_sock_close(struct sock *sk)
> > > > > > > > > > > >
> > > > > > > > > > > >         case BT_CONNECTED:
> > > > > > > > > > > >         case BT_CONFIG:
> > > > > > > > > > > > -               if (sco_pi(sk)->conn->hcon) {
> > > > > > > > > > > > -                       sk->sk_state = BT_DISCONN;
> > > > > > > > > > > > -                       sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
> > > > > > > > > > > > -                       sco_conn_lock(sco_pi(sk)->conn);
> > > > > > > > > > > > -                       hci_conn_drop(sco_pi(sk)->conn->hcon);
> > > > > > > > > > > > -                       sco_pi(sk)->conn->hcon = NULL;
> > > > > > > > > > > > -                       sco_conn_unlock(sco_pi(sk)->conn);
> > > > > > > > > > > > -               } else
> > > > > > > > > > > > -                       sco_chan_del(sk, ECONNRESET);
> > > > > > > > > > > > -               break;
> > > > > > > > > > > > -
> > > > > > > > > > > >         case BT_CONNECT2:
> > > > > > > > > > > >         case BT_CONNECT:
> > > > > > > > > > > >         case BT_DISCONN:
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.46.1
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > --
> > > > > > > > > > > Luiz Augusto von Dentz
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > Luiz Augusto von Dentz
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Luiz Augusto von Dentz
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Luiz Augusto von Dentz
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Luiz Augusto von Dentz
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Luiz Augusto von Dentz
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[-- Attachment #2: v1-0001-Bluetooth-SCO-Fix-UAF-on-sco_sock_timeout.patch --]
[-- Type: text/x-patch, Size: 3372 bytes --]

From 018604f4be8f1e769a358b1e7bf93e1c2cc83e28 Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 22 Oct 2024 12:31:08 -0400
Subject: [PATCH v1] Bluetooth: SCO: Fix UAF on sco_sock_timeout

conn->sk maybe have been unlinked/freed while waiting for sco_conn_lock
so this checks if the conn->sk is still valid by checking if it part of
sco_sk_list.

Reported-by: syzbot+4c0d0c4cde787116d465@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4c0d0c4cde787116d465
Fixes: ba316be1b6a0 ("Bluetooth: schedule SCO timeouts with delayed_work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/af_bluetooth.c      | 22 ++++++++++++++++++++++
 net/bluetooth/sco.c               | 18 ++++++++++++------
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 5d655e109b2c..f66bc85c6411 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -403,6 +403,7 @@ int  bt_sock_register(int proto, const struct net_proto_family *ops);
 void bt_sock_unregister(int proto);
 void bt_sock_link(struct bt_sock_list *l, struct sock *s);
 void bt_sock_unlink(struct bt_sock_list *l, struct sock *s);
+bool bt_sock_linked(struct bt_sock_list *l, struct sock *s);
 struct sock *bt_sock_alloc(struct net *net, struct socket *sock,
 			   struct proto *prot, int proto, gfp_t prio, int kern);
 int  bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index e39fba5565c5..0b4d0a8bd361 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -185,6 +185,28 @@ void bt_sock_unlink(struct bt_sock_list *l, struct sock *sk)
 }
 EXPORT_SYMBOL(bt_sock_unlink);
 
+bool bt_sock_linked(struct bt_sock_list *l, struct sock *s)
+{
+	struct sock *sk;
+
+	if (!l || !s)
+		return false;
+
+	read_lock(&l->lock);
+
+	sk_for_each(sk, &l->head) {
+		if (s == sk) {
+			read_unlock(&l->lock);
+			return true;
+		}
+	}
+
+	read_unlock(&l->lock);
+
+	return false;
+}
+EXPORT_SYMBOL(bt_sock_linked);
+
 void bt_accept_enqueue(struct sock *parent, struct sock *sk, bool bh)
 {
 	const struct cred *old_cred;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index a5ac160c592e..1c7252a36866 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -76,6 +76,16 @@ struct sco_pinfo {
 #define SCO_CONN_TIMEOUT	(HZ * 40)
 #define SCO_DISCONN_TIMEOUT	(HZ * 2)
 
+static struct sock *sco_sock_hold(struct sco_conn *conn)
+{
+	if (!conn || !bt_sock_linked(&sco_sk_list, conn->sk))
+		return NULL;
+
+	sock_hold(conn->sk);
+
+	return conn->sk;
+}
+
 static void sco_sock_timeout(struct work_struct *work)
 {
 	struct sco_conn *conn = container_of(work, struct sco_conn,
@@ -87,9 +97,7 @@ static void sco_sock_timeout(struct work_struct *work)
 		sco_conn_unlock(conn);
 		return;
 	}
-	sk = conn->sk;
-	if (sk)
-		sock_hold(sk);
+	sk = sco_sock_hold(conn);
 	sco_conn_unlock(conn);
 
 	if (!sk)
@@ -194,9 +202,7 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	/* Kill socket */
 	sco_conn_lock(conn);
-	sk = conn->sk;
-	if (sk)
-		sock_hold(sk);
+	sk = sco_sock_hold(conn);
 	sco_conn_unlock(conn);
 
 	if (sk) {
-- 
2.47.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-10-22 19:51 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-16 11:20 [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
     [not found] <000000000000797bd1060a457c08@google.com>
2023-12-06  3:58 ` syzbot
  -- strict thread matches above, loose matches on Subject: below --
2024-10-01 19:49 [PATCH v1] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-01 20:13 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-02 18:26 [PATCH v2] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-02 18:46 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-02 19:19 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-02 19:37 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-02 19:46 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-02 20:05 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-02 20:46 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-02 23:16 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-03 15:38 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-03 15:55 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-03 16:32 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-03 16:53 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-03 19:21 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-03 19:44 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-03 20:06   ` Luiz Augusto von Dentz
2024-10-04 16:06 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-04 16:34 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-04 17:24 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-04 17:40 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-07 17:16 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-07 17:33 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-07 20:54 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-07 21:15 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-22 16:44 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-22 17:15 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot
2024-10-22 19:19 [PATCH v3] Bluetooth: SCO: Use disable_delayed_work_sync Luiz Augusto von Dentz
2024-10-22 19:51 ` [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in sco_sock_timeout syzbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).