lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABBYNZJ73acaaO7m8eEk=PSGrSHE+ZxKUEnm3spaDjULRT7ifg@mail.gmail.com>
Date: Fri, 20 Sep 2024 11:07:05 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Edward Adam Davis <eadavis@...com>
Cc: syzbot+c12e2f941af1feb5632c@...kaller.appspotmail.com, 
	johan.hedberg@...il.com, linux-bluetooth@...r.kernel.org, 
	linux-kernel@...r.kernel.org, marcel@...tmann.org, 
	syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] Bluetooth/l2cap: Fix uaf in l2cap_connect

Hi Edward,

On Tue, Sep 10, 2024 at 4:56 PM Luiz Augusto von Dentz
<luiz.dentz@...il.com> wrote:
>
> Hi Edward,
>
> On Sun, Sep 8, 2024 at 3:22 AM Edward Adam Davis <eadavis@...com> wrote:
> >
> > [Syzbot reported]
> > BUG: KASAN: slab-use-after-free in l2cap_connect.constprop.0+0x10d8/0x1270 net/bluetooth/l2cap_core.c:3949
> > Read of size 8 at addr ffff8880241e9800 by task kworker/u9:0/54
> >
> > CPU: 0 UID: 0 PID: 54 Comm: kworker/u9:0 Not tainted 6.11.0-rc6-syzkaller-00268-g788220eee30d #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024
> > Workqueue: hci2 hci_rx_work
> > Call Trace:
> >  <TASK>
> >  __dump_stack lib/dump_stack.c:93 [inline]
> >  dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:119
> >  print_address_description mm/kasan/report.c:377 [inline]
> >  print_report+0xc3/0x620 mm/kasan/report.c:488
> >  kasan_report+0xd9/0x110 mm/kasan/report.c:601
> >  l2cap_connect.constprop.0+0x10d8/0x1270 net/bluetooth/l2cap_core.c:3949
> >  l2cap_connect_req net/bluetooth/l2cap_core.c:4080 [inline]
> >  l2cap_bredr_sig_cmd net/bluetooth/l2cap_core.c:4772 [inline]
> >  l2cap_sig_channel net/bluetooth/l2cap_core.c:5543 [inline]
> >  l2cap_recv_frame+0xf0b/0x8eb0 net/bluetooth/l2cap_core.c:6825
> >  l2cap_recv_acldata+0x9b4/0xb70 net/bluetooth/l2cap_core.c:7514
> >  hci_acldata_packet net/bluetooth/hci_core.c:3791 [inline]
> >  hci_rx_work+0xaab/0x1610 net/bluetooth/hci_core.c:4028
> >  process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
> >  process_scheduled_works kernel/workqueue.c:3312 [inline]
> >  worker_thread+0x6c8/0xed0 kernel/workqueue.c:3389
> >  kthread+0x2c1/0x3a0 kernel/kthread.c:389
> >  ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> > ...
> >
> > Freed by task 5245:
> >  kasan_save_stack+0x33/0x60 mm/kasan/common.c:47
> >  kasan_save_track+0x14/0x30 mm/kasan/common.c:68
> >  kasan_save_free_info+0x3b/0x60 mm/kasan/generic.c:579
> >  poison_slab_object+0xf7/0x160 mm/kasan/common.c:240
> >  __kasan_slab_free+0x32/0x50 mm/kasan/common.c:256
> >  kasan_slab_free include/linux/kasan.h:184 [inline]
> >  slab_free_hook mm/slub.c:2256 [inline]
> >  slab_free mm/slub.c:4477 [inline]
> >  kfree+0x12a/0x3b0 mm/slub.c:4598
> >  l2cap_conn_free net/bluetooth/l2cap_core.c:1810 [inline]
> >  kref_put include/linux/kref.h:65 [inline]
> >  l2cap_conn_put net/bluetooth/l2cap_core.c:1822 [inline]
> >  l2cap_conn_del+0x59d/0x730 net/bluetooth/l2cap_core.c:1802
> >  l2cap_connect_cfm+0x9e6/0xf80 net/bluetooth/l2cap_core.c:7241
> >  hci_connect_cfm include/net/bluetooth/hci_core.h:1960 [inline]
> >  hci_conn_failed+0x1c3/0x370 net/bluetooth/hci_conn.c:1265
> >  hci_abort_conn_sync+0x75a/0xb50 net/bluetooth/hci_sync.c:5583
> >  abort_conn_sync+0x197/0x360 net/bluetooth/hci_conn.c:2917
> >  hci_cmd_sync_work+0x1a4/0x410 net/bluetooth/hci_sync.c:328
> >  process_one_work+0x9c5/0x1b40 kernel/workqueue.c:3231
> >  process_scheduled_works kernel/workqueue.c:3312 [inline]
> >  worker_thread+0x6c8/0xed0 kernel/workqueue.c:3389
> >  kthread+0x2c1/0x3a0 kernel/kthread.c:389
> >  ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> >
> > [Analysis]
> > There was a data race when accessing conn in hci_rx_work and hci_cmd_sync_work.
> > This is because the hci dev lock was prematurely exited when executing
> > hci_acldata_macket() in hci_rx_work, which resulted in it being released
> > by hci_cmd_sync_work when accessing conn outside the lock.
> >
> > Reported-and-tested-by: syzbot+c12e2f941af1feb5632c@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=c12e2f941af1feb5632c
> > Signed-off-by: Edward Adam Davis <eadavis@...com>
> > ---
> >  net/bluetooth/hci_core.c   | 3 ++-
> >  net/bluetooth/l2cap_core.c | 2 --
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index f25a21f532aa..4f7b45bb863f 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -3776,18 +3776,19 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> >
> >         hci_dev_lock(hdev);
> >         conn = hci_conn_hash_lookup_handle(hdev, handle);
> > -       hci_dev_unlock(hdev);
> >
> >         if (conn) {
> >                 hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF);
> >
> >                 /* Send to upper protocol */
> >                 l2cap_recv_acldata(conn, skb, flags);
> > +               hci_dev_unlock(hdev);
> >                 return;
> >         } else {
> >                 bt_dev_err(hdev, "ACL packet for unknown connection handle %d",
> >                            handle);
> >         }
> > +       hci_dev_unlock(hdev);
>
> This is sort of risky, we shouldn't be calling this deep into the
> stack with hci_dev_lock held.
>
> >
> >         kfree_skb(skb);
> >  }
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 9988ba382b68..b948b0a3b2f2 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -4072,10 +4072,8 @@ static int l2cap_connect_req(struct l2cap_conn *conn,
> >         if (cmd_len < sizeof(struct l2cap_conn_req))
> >                 return -EPROTO;
> >
> > -       hci_dev_lock(hdev);
> >         if (hci_dev_test_flag(hdev, HCI_MGMT))
> >                 mgmt_device_connected(hdev, hcon, NULL, 0);
> > -       hci_dev_unlock(hdev);
>
> So this might explain why things gets freed while processing the
> request, we are locking to call mgmt_device_connected which I suspect
> is no longer needed ever since:
>
> commit db11223571d489d1aab575a4ac4b7352d2d54e2f
> Author: Luiz Augusto von Dentz <luiz.von.dentz@...el.com>
> Date:   Tue Oct 25 14:12:58 2022 -0700
>
>     Bluetooth: btusb: Default CONFIG_BT_HCIBTUSB_POLL_SYNC=y
>
>     poll_sync has been proven to fix races of USB data and event endpoints
>     so this enables it by default.
>
>     Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@...el.com>
>     Tested-by: Tedd Ho-Jeong An <tedd.an@...el.com>
>
> Anyway syzbot don't use btusb so I think this might be due some
> command pending that the emulator is not responding and instead
> sending data, and then there is the issue that 7b064edae38d
> ("Bluetooth: Fix authentication if acl data comes before remote
> feature evt") attempted to fix which I think it actually made it worse
> by moving the call to mgmt_device_connected into l2cap_core.c it sort
> move the problem but didn't fix the actual problem.
>
> Maybe something like the following would be a better approach:
>
> https://gist.github.com/Vudentz/121a15fa4391b2b1f6c7e8d420a6846e

Any comments? Are you still planning to work on this?

> >
> >         l2cap_connect(conn, cmd, data, L2CAP_CONN_RSP);
> >         return 0;
> > --
> > 2.43.0
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ