diff mbox series

Bluetooth: L2CAP: Fix slab-use-after-free in l2cap_send_cmd

Message ID 20240426072006.358802-1-iam@sung-woo.kim
State New
Headers show
Series Bluetooth: L2CAP: Fix slab-use-after-free in l2cap_send_cmd | expand

Commit Message

Sungwoo Kim April 26, 2024, 7:20 a.m. UTC
Hello, could you review a bug and its fix?

This is a racy bug. Call stack is as below:

[free]
l2cap_conn_del
┌ mutex_lock(&conn->chan_lock);
│ foreach chan in conn->chan_l:            ... (2)
│   l2cap_chan_put(chan);
│     l2cap_chan_destroy
│       kfree(chan)                        ... (3)
└ mutex_unlock(&conn->chan_lock);

[use]
l2cap_bredr_sig_cmd
  l2cap_connect
  ┌ mutex_lock(&conn->chan_lock);
  │ chan = pchan->ops->new_connection(pchan); // alloc chan
  │ __l2cap_chan_add(conn, chan);
  │   l2cap_chan_hold(chan);
  │   list_add(&chan->list, &conn->chan_l);   ... (1)
  └ mutex_unlock(&conn->chan_lock);
    chan->conf_state // uaf at chan           ... (4)

To fix this, this patch holds and locks the l2cap channel.

Thanks,
Sungwoo.

==================================================================
BUG: KASAN: slab-use-after-free in l2cap_send_cmd+0x5dc/0x830 net/bluetooth/l2cap_core.c:968
Read of size 1 at addr ffff88810b62c274 by task kworker/0:1/10

CPU: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.8.0+ #61
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Workqueue: events l2cap_info_timeout
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x85/0xb0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x18f/0x560 mm/kasan/report.c:488
 kasan_report+0xd7/0x110 mm/kasan/report.c:601
 __asan_report_load1_noabort+0x18/0x20 mm/kasan/report_generic.c:378
 l2cap_send_cmd+0x5dc/0x830 net/bluetooth/l2cap_core.c:968
 l2cap_send_conn_req net/bluetooth/l2cap_core.c:1286 [inline]
 l2cap_start_connection+0x25e/0x530 net/bluetooth/l2cap_core.c:1514
 l2cap_conn_start+0x952/0xe60 net/bluetooth/l2cap_core.c:1661
 l2cap_info_timeout+0x5f/0x90 net/bluetooth/l2cap_core.c:1807
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
 worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
 kthread+0x2a9/0x340 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
 </TASK>

Allocated by task 290:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_alloc_info+0x3c/0x50 mm/kasan/generic.c:575
 poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
 __kasan_kmalloc+0xa2/0xc0 mm/kasan/common.c:387
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 __do_kmalloc_node mm/slub.c:3981 [inline]
 __kmalloc+0x228/0x4d0 mm/slub.c:3994
 kmalloc include/linux/slab.h:594 [inline]
 kzalloc include/linux/slab.h:711 [inline]
 hci_alloc_dev_priv+0x30/0x1ea0 net/bluetooth/hci_core.c:2468
 hci_alloc_dev include/net/bluetooth/hci_core.h:1651 [inline]
 __vhci_create_device drivers/bluetooth/hci_vhci.c:406 [inline]
 vhci_create_device+0x10e/0x710 drivers/bluetooth/hci_vhci.c:480
 vhci_get_user drivers/bluetooth/hci_vhci.c:537 [inline]
 vhci_write+0x398/0x460 drivers/bluetooth/hci_vhci.c:617
 call_write_iter include/linux/fs.h:2087 [inline]
 new_sync_write fs/read_write.c:497 [inline]
 vfs_write+0x9ee/0xd40 fs/read_write.c:590
 ksys_write+0x103/0x1f0 fs/read_write.c:643
 __do_sys_write fs/read_write.c:655 [inline]
 __se_sys_write fs/read_write.c:652 [inline]
 __x64_sys_write+0x84/0xa0 fs/read_write.c:652
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x84/0x120 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x6e/0x76

Freed by task 1158:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x44/0x50 mm/kasan/generic.c:589
 poison_slab_object+0x11a/0x190 mm/kasan/common.c:240
 __kasan_slab_free+0x3b/0x60 mm/kasan/common.c:256
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x106/0x2e0 mm/slub.c:4409
 hci_release_dev+0x1114/0x1250 net/bluetooth/hci_core.c:2799
 bt_host_release+0x77/0x90 net/bluetooth/hci_sysfs.c:94
 device_release+0xa4/0x1d0
 kobject_cleanup lib/kobject.c:689 [inline]
 kobject_release lib/kobject.c:720 [inline]
 kref_put include/linux/kref.h:65 [inline]
 kobject_put+0x1f1/0x410 lib/kobject.c:737
 put_device+0x28/0x40 drivers/base/core.c:3747
 hci_free_dev+0x25/0x30 net/bluetooth/hci_core.c:2594
 vhci_release+0x91/0xe0 drivers/bluetooth/hci_vhci.c:675
 __fput+0x35b/0x740 fs/file_table.c:376
 ____fput+0x1e/0x30 fs/file_table.c:404
 task_work_run+0x1d9/0x270 kernel/task_work.c:180
 exit_task_work include/linux/task_work.h:38 [inline]
 do_exit+0x823/0x23e0 kernel/exit.c:871
 do_group_exit+0x1f1/0x2b0 kernel/exit.c:1020
 get_signal+0x1387/0x14d0 kernel/signal.c:2893
 arch_do_signal_or_restart+0x3b/0x650 arch/x86/kernel/signal.c:310
 exit_to_user_mode_loop kernel/entry/common.c:105 [inline]
 exit_to_user_mode_prepare include/linux/entry-common.h:328 [inline]
 __syscall_exit_to_user_mode_work kernel/entry/common.c:201 [inline]
 syscall_exit_to_user_mode+0x71/0x180 kernel/entry/common.c:212
 do_syscall_64+0x90/0x120 arch/x86/entry/common.c:89
 entry_SYSCALL_64_after_hwframe+0x6e/0x76

Last potentially related work creation:
 kasan_save_stack+0x2f/0x50 mm/kasan/common.c:47
 __kasan_record_aux_stack+0xc6/0xe0 mm/kasan/generic.c:551
 kasan_record_aux_stack_noalloc+0xf/0x20 mm/kasan/generic.c:561
 insert_work+0x3a/0x1d0 kernel/workqueue.c:1653
 __queue_work+0xa5b/0xdc0 kernel/workqueue.c:1806
 queue_work_on+0x74/0xa0 kernel/workqueue.c:1837
 queue_work include/linux/workqueue.h:548 [inline]
 hci_recv_frame+0x375/0x490 net/bluetooth/hci_core.c:2947
 hci_reset_dev+0x126/0x170 net/bluetooth/hci_core.c:2904
 hci_ncmd_timeout+0xb1/0xd0 net/bluetooth/hci_core.c:1525
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
 worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
 kthread+0x2a9/0x340 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243

Second to last potentially related work creation:
 kasan_save_stack+0x2f/0x50 mm/kasan/common.c:47
 __kasan_record_aux_stack+0xc6/0xe0 mm/kasan/generic.c:551
 kasan_record_aux_stack_noalloc+0xf/0x20 mm/kasan/generic.c:561
 insert_work kernel/workqueue.c:1653 [inline]
 __queue_work+0x8cf/0xdc0 kernel/workqueue.c:1802
 delayed_work_timer_fn+0x6a/0x90 kernel/workqueue.c:1931
 call_timer_fn+0x44/0x240 kernel/time/timer.c:1700
 expire_timers kernel/time/timer.c:1746 [inline]
 __run_timers+0x63a/0x870 kernel/time/timer.c:2038
 run_timer_softirq+0x26/0x40 kernel/time/timer.c:2051
 __do_softirq+0x180/0x523 kernel/softirq.c:553

The buggy address belongs to the object at ffff88810b62c000
 which belongs to the cache kmalloc-8k of size 8192
The buggy address is located 628 bytes inside of
 freed 8192-byte region [ffff88810b62c000, ffff88810b62e000)

The buggy address belongs to the physical page:
page:000000007f8eb4fe refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10b628
head:000000007f8eb4fe order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
anon flags: 0x17ffffc0000840(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
page_type: 0xffffffff()
raw: 0017ffffc0000840 ffff888100042280 0000000000000000 0000000000000001
raw: 0000000000000000 0000000000020002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff88810b62c100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88810b62c180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88810b62c200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                             ^
 ffff88810b62c280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88810b62c300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

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

Comments

Sungwoo Kim April 26, 2024, 7:31 a.m. UTC | #1
Wrong call trace was attached. The correct one is below.
Sorry!

==================================================================
BUG: KASAN: slab-use-after-free in instrument_atomic_read include/linux/instrumented.h:68 [inline]
BUG: KASAN: slab-use-after-free in _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
BUG: KASAN: slab-use-after-free in l2cap_connect+0xa67/0x11a0 net/bluetooth/l2cap_core.c:4260
Read of size 8 at addr ffff88810bf040a0 by task kworker/u3:1/311

CPU: 0 PID: 311 Comm: kworker/u3:1 Not tainted 6.8.0+ #61
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Workqueue: hci0 hci_rx_work
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x85/0xb0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x18f/0x560 mm/kasan/report.c:488
 kasan_report+0xd7/0x110 mm/kasan/report.c:601
 kasan_check_range+0x262/0x2f0 mm/kasan/generic.c:189
 __kasan_check_read+0x15/0x20 mm/kasan/shadow.c:31
 instrument_atomic_read include/linux/instrumented.h:68 [inline]
 _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
 l2cap_connect+0xa67/0x11a0 net/bluetooth/l2cap_core.c:4260
 l2cap_bredr_sig_cmd+0x17fe/0x9a70
 l2cap_sig_channel net/bluetooth/l2cap_core.c:6539 [inline]
 l2cap_recv_frame+0x82e/0x86a0 net/bluetooth/l2cap_core.c:7818
 l2cap_recv_acldata+0x379/0xbe0 net/bluetooth/l2cap_core.c:8536
 hci_acldata_packet net/bluetooth/hci_core.c:3876 [inline]
 hci_rx_work+0x64b/0xcb0 net/bluetooth/hci_core.c:4111
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
 worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
 kthread+0x2a9/0x340 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
 </TASK>

Allocated by task 311:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_alloc_info+0x3c/0x50 mm/kasan/generic.c:575
 poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
 __kasan_kmalloc+0xa2/0xc0 mm/kasan/common.c:387
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 kmalloc_trace+0x1c9/0x390 mm/slub.c:4012
 kmalloc include/linux/slab.h:590 [inline]
 kzalloc include/linux/slab.h:711 [inline]
 l2cap_chan_create+0x59/0xc80 net/bluetooth/l2cap_core.c:466
 l2cap_sock_alloc net/bluetooth/l2cap_sock.c:1849 [inline]
 l2cap_sock_new_connection_cb+0x14d/0x2a0 net/bluetooth/l2cap_sock.c:1457
 l2cap_connect+0x329/0x11a0 net/bluetooth/l2cap_core.c:4176
 l2cap_bredr_sig_cmd+0x17fe/0x9a70
 l2cap_sig_channel net/bluetooth/l2cap_core.c:6539 [inline]
 l2cap_recv_frame+0x82e/0x86a0 net/bluetooth/l2cap_core.c:7818
 l2cap_recv_acldata+0x379/0xbe0 net/bluetooth/l2cap_core.c:8536
 hci_acldata_packet net/bluetooth/hci_core.c:3876 [inline]
 hci_rx_work+0x64b/0xcb0 net/bluetooth/hci_core.c:4111
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
 worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
 kthread+0x2a9/0x340 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243

Freed by task 66:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x30/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x44/0x50 mm/kasan/generic.c:589
 poison_slab_object+0x11a/0x190 mm/kasan/common.c:240
 __kasan_slab_free+0x3b/0x60 mm/kasan/common.c:256
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x106/0x2e0 mm/slub.c:4409
 l2cap_chan_destroy net/bluetooth/l2cap_core.c:509 [inline]
 kref_put include/linux/kref.h:65 [inline]
 l2cap_chan_put+0x1e7/0x2b0 net/bluetooth/l2cap_core.c:533
 l2cap_conn_del+0x38e/0x5f0 net/bluetooth/l2cap_core.c:1929
 l2cap_connect_cfm+0xc2/0x11e0 net/bluetooth/l2cap_core.c:8254
 hci_connect_cfm include/net/bluetooth/hci_core.h:1986 [inline]
 hci_conn_failed+0x202/0x370 net/bluetooth/hci_conn.c:1289
 hci_abort_conn_sync+0x913/0xae0 net/bluetooth/hci_sync.c:5359
 abort_conn_sync+0xda/0x110 net/bluetooth/hci_conn.c:2988
 hci_cmd_sync_work+0x20d/0x3e0 net/bluetooth/hci_sync.c:306
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
 worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
 kthread+0x2a9/0x340 kernel/kthread.c:388
 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243

The buggy address belongs to the object at ffff88810bf04000
 which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 160 bytes inside of
 freed 1024-byte region [ffff88810bf04000, ffff88810bf04400)

The buggy address belongs to the physical page:
page:00000000567b7faa refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10bf04
head:00000000567b7faa order:2 entire_mapcount:0 nr_pages_mapped:0 pincount:0
anon flags: 0x17ffffc0000840(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
page_type: 0xffffffff()
raw: 0017ffffc0000840 ffff888100041dc0 0000000000000000 0000000000000001
raw: 0000000000000000 0000000080080008 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff88810bf03f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff88810bf04000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88810bf04080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                               ^
 ffff88810bf04100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff88810bf04180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Markus Elfring April 26, 2024, 8:25 a.m. UTC | #2
I prefer that you would put recipient specifications also into the message field “To”
(besides “Cc”).


> Hello, could you review a bug and its fix?

I suggest to omit such a question from better change descriptions.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9-rc5#n45


…
> To fix this, this patch holds and locks the l2cap channel.

Please choose a corresponding imperative wording.


You would probably like to improve your patch approach further
so that provided data will be kept consistent.
https://lore.kernel.org/lkml/20240426073142.363876-1-iam@sung-woo.kim/

Regards,
Markus
Sungwoo Kim April 26, 2024, 9:35 a.m. UTC | #3
On Fri, Apr 26, 2024 at 5:16 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Fri, Apr 26, 2024 at 03:20:05AM -0400, Sungwoo Kim wrote:
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 84fc70862..a8f414ab8 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -3953,6 +3953,9 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
> >       if (!chan)
> >               goto response;
> >
> > +     l2cap_chan_hold(chan);
> > +     l2cap_chan_lock(chan);
> > +
> >       /* For certain devices (ex: HID mouse), support for authentication,
> >        * pairing and bonding is optional. For such devices, inorder to avoid
> >        * the ACL alive for too long after L2CAP disconnection, reset the ACL
> > @@ -4041,6 +4044,11 @@ static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
> >               chan->num_conf_req++;
> >       }
> >
> > +     if (chan) {
> > +             l2cap_chan_unlock(chan);
> > +             l2cap_chan_put(chan);
> > +     }
> > +
> >       return chan;
>         ^^^^^^^^^^^^
> This doesn't fix the bug because we're returning chan.
>
> As soon as you call l2cap_chan_put() then chan will be freed by in the
> other thread which is doing l2cap_conn_del() resulting in a use after
> free in the caller.

Thank you for pointing this out.
No caller uses the return value of l2cap_connect() if the kernel
versions >= v6.9.
So, l2cap_connect() can return void.

One caller uses the return value of l2cap_connect() in v4.19 <= the
kernel versions <= v6.8.
In this case, the caller should unlock and put a channel.

Question: Can different patches be applied for different versions like
the above?

Thanks,
Sungwoo Kim.
Sungwoo Kim April 26, 2024, 9:48 a.m. UTC | #4
On Fri, Apr 26, 2024 at 5:38 AM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Please just ignore Markus.  A lot of people have asked him to stop
> commenting on commit messages but he doesn't listen.  Here is a message
> from yesterday:
>
> https://lore.kernel.org/all/2024042547-shimmy-guileless-c7f2@gregkh/
>
> 1) It doesn't matter at all if there is anyone in the To: header.
> 2) You are allowed to ask questions.
> 3) Yes, the commit message will need to be changed but first fix the bug
>    and then we can worry about the commit message.
>
> regards,
> dan carpenter
>
>

Thank you for letting me know :)
Dan Carpenter April 26, 2024, 10:24 a.m. UTC | #5
On Fri, Apr 26, 2024 at 05:35:01AM -0400, Sungwoo Kim wrote:
> > > +
> > >       return chan;
> >         ^^^^^^^^^^^^
> > This doesn't fix the bug because we're returning chan.
> >
> > As soon as you call l2cap_chan_put() then chan will be freed by in the
> > other thread which is doing l2cap_conn_del() resulting in a use after
> > free in the caller.
> 
> Thank you for pointing this out.
> No caller uses the return value of l2cap_connect() if the kernel
> versions >= v6.9.
> So, l2cap_connect() can return void.
> 
> One caller uses the return value of l2cap_connect() in v4.19 <= the
> kernel versions <= v6.8.
> In this case, the caller should unlock and put a channel.
> 
> Question: Can different patches be applied for different versions like
> the above?

Ah...  Very good.  I assumed it was used.  The the commit which stopped
using the return value, commit e7b02296fb40 ("Bluetooth: Remove BT_HS"),
has been back ported to earlier kernels as well.

Generally, we just write code against the latest kernel and worry about
backports as a separate issue.  We sometimes re-write patches slightly
if that's necessary for the backport.

I'm not an expert in bluetooth, but I think your patch seems correct.
Let's make l2cap_connect() void as well.  Wait for a day or two for
other comments and then send a v2 patch.
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

Here is how you write the commit message:
========================================================================

[PATCH v2] Bluetooth: L2CAP: Fix slab-use-after-free in l2cap_connect()

KASAN detected a use after free in l2cap_send_cmd().
BUG: KASAN: slab-use-after-free in l2cap_send_cmd+0x5dc/0x830 net/bluetooth/l2cap_core.c:968

[free]
l2cap_conn_del
┌ mutex_lock(&conn->chan_lock);
│ foreach chan in conn->chan_l:            ... (2)
│   l2cap_chan_put(chan);
│     l2cap_chan_destroy
│       kfree(chan)                        ... (3)  <-- chan freed
└ mutex_unlock(&conn->chan_lock);

[use]
l2cap_bredr_sig_cmd
  l2cap_connect
  ┌ mutex_lock(&conn->chan_lock);
  │ chan = pchan->ops->new_connection(pchan);  <-- allocates chan
  │ __l2cap_chan_add(conn, chan);
  │   l2cap_chan_hold(chan);
  │   list_add(&chan->list, &conn->chan_l);  ... (1)
  └ mutex_unlock(&conn->chan_lock);
    chan->conf_state			     ... (4)  <-- use after free

To fix this, this patch holds and locks the l2cap channel.

Also make the l2cap_connect() return type void.  Nothing is using the
returned value but it is ugly to return a potentially freed pointer.
Making it void will help with backports because earlier kernels did use
the return value.  Now the compile will break for kernels where this
patch is not a complete fix.

Fixes: 73ffa904b782 ("Bluetooth: Move conf_{req,rsp} stuff to struct l2cap_chan")
Signed-off-by:
diff mbox series

Patch

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 84fc70862..a8f414ab8 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3953,6 +3953,9 @@  static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 	if (!chan)
 		goto response;
 
+	l2cap_chan_hold(chan);
+	l2cap_chan_lock(chan);
+
 	/* For certain devices (ex: HID mouse), support for authentication,
 	 * pairing and bonding is optional. For such devices, inorder to avoid
 	 * the ACL alive for too long after L2CAP disconnection, reset the ACL
@@ -4041,6 +4044,11 @@  static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
 		chan->num_conf_req++;
 	}
 
+	if (chan) {
+		l2cap_chan_unlock(chan);
+		l2cap_chan_put(chan);
+	}
+
 	return chan;
 }