Request for Comments
Summary
=======
We have found a memory leak in the Bluetooth L2CAP subsystem and fixed
it, but this opened the doors to use-after-free problems, which are
not completely fixed yet. This patch series present a way to reproduce
it, the proposed fix and the status. There is more detailed
documentation in [1].
Memory Leak Overview
====================
We have found a memory leak in the L2CAP layer of Linux's Bluetooth
Subsystem, the same as reported by syzbot in "[syzbot] [bluetooth?]
memory leak in hci_conn_add (2)" (2023-09-02 23:25:00 -0700) [19].
We can reproduce it. When, in a loop, a user-mode program:
1. Opens a Bluetooth socket at the L2CAP layer:
sockfd = socket(AF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP);
2. Set a timeout on the socket:
timeout.tv_sec = 2;
timeout.tv_usec = 0;
err = setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO, &timeout, sizeof(timeout));
3. Connect to a specific Bluetooth address:
struct sockaddr_l2 ble_addr;
memset(&ble_addr, 0, sizeof(ble_addr));
ble_addr.l2_family = AF_BLUETOOTH;
ble_addr.l2_psm = htobs(0x80 /* L2CAP_PSM_LE_DYN_START */);
// l2_bdaddr=00:0a:07:a3:22:00
ble_addr.l2_bdaddr = *(bdaddr_t*)("\x00\x0a\x07\xa3\x22\x00");
ble_addr.l2_cid = htobs(0);
ble_addr.l2_bdaddr_type = BDADDR_LE_PUBLIC;
err = connect(sockfd, (struct sockaddr*)&ble_addr, sizeof(ble_addr));
and when the user program does those three steps in a loop, then the
kernel leaks the "struct l2cap_conn" [5] and "struct hci_conn" [6]
objects allocated in "l2cap_conn_add()" [2] and "hci_conn_add()" [3].
Those objects are never freed.
The "ble-memleak-repro" program reproduces the memory leak, if the
kernel is not patched. Its source is in
"package/ble-memleak-repro/ble-memleak-repro.c" [18].
Memory Leak Fix
===============
We have fixed the memory leak, see the patch series in
"patches/linux/":
1. The first patch
"0001-ARM-dts-stm32-Add-Bluetooth-usart2-feature-on-stm32m.patch" [11]
enables Bluetooth on the DK2.
2. The second patch
"0002-Bluetooth-add-many-traces-for-allocation-free-refcou.patch" [12]
adds many dynamic debug traces that help understand the kernel
behaviour and freeing problems.
3. Patches
"0003-Bluetooth-L2CAP-use-refcount-on-struct-l2cap_chan-co.patch" [13]
and
"0004-Bluetooth-L2CAP-free-the-leaking-struct-l2cap_conn.patch" [14]
fix the memory leak.
4. Patches
"0005-Bluetooth-introduce-hci_conn_free-for-better-structu.patch" [15],
"0006-Bluetooth-L2CAP-inc-refcount-if-reuse-struct-l2cap_c.patch" [16]
and
"0007-Bluetooth-unlink-objects-to-avoid-use-after-free.patch" [17]
fixes the use-after-free that appears when the "struct l2cap_conn"
[5] and "struct hci_conn" [6] objects are freed.
The commit messages explain why the commit is needed, which problem
the commit solves, and how.
The first and second patches are present for the memory leak
reproduction only, they should not be part of a final fix.
Patch Status
============
As of writing, the memory leak is fixed. The fix opened the door to
other problems, especially use-after-free, sometimes followed by
crashes due to NULL dereferences. We think there are weak references
(i.e. pointers that don't increment the refcounter) previously
pointing to memory that was never freed, but now is freed. On kernels
v5.13 and v5.15, patches 0005, 0006 and 0007 seem to fix the
use-after-free and NULL dereferences, but not on kernel v6.5 yet.
Reproducing with Buildroot
==========================
We have reproduced and fixed the memory leak with Buildroot [7] and a
"ble-memleak-repro" test application on an ST's Discovery Kit DK2 [4].
The "ble-memleak-repro" repository [1] contains the sources of a
complete external Buildroot customisation [8], with the patches, a
README, and more explanations to reproduce the problem with Buildroot
on a DK2.
References:
===========
- [1]: <https://gitlab.com/essensium-mind/ble-memleak-repro.git>
"ble-memleak-repro repository"
- [2]: <https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/l2cap_core.c#L7833>
"l2cap_conn_add()"
- [3]: <https://elixir.bootlin.com/linux/v6.5/source/net/bluetooth/hci_conn.c#L986>
"hci_conn_add()"
- [4]: <https://www.st.com/content/st_com/en/products/evaluation-tools/product-evaluation-tools/mcu-mpu-eval-tools/stm32-mcu-mpu-eval-tools/stm32-discovery-kits/stm32mp157c-dk2.html>
"STM32MP157C-DK2 (DK2)"
- [5]: <https://elixir.bootlin.com/linux/v6.5/source/include/net/bluetooth/l2cap.h#L674>
"struct l2cap_conn"
- [6]: <https://elixir.bootlin.com/linux/v6.5/source/include/net/bluetooth/hci_core.h#L685>
"struct hci_conn"
- [7]: <https://buildroot.org/>
"Buildroot is a tool to generate embedded Linux systems"
- [8]: <https://buildroot.org/downloads/manual/manual.html#outside-br-custom>
"9.2. Keeping customizations outside of Buildroot"
- [11]: <patches/linux/0001-ARM-dts-stm32-Add-Bluetooth-usart2-feature-on-stm32m.patch>
"0001-ARM-dts-stm32-Add-Bluetooth-usart2-feature-on-stm32m.patch"
- [12]: <patches/linux/0002-Bluetooth-add-many-traces-for-allocation-free-refcou.patch>
"0002-Bluetooth-add-many-traces-for-allocation-free-refcou.patch"
- [13]: <patches/linux/0003-Bluetooth-L2CAP-use-refcount-on-struct-l2cap_chan-co.patch>
"0003-Bluetooth-L2CAP-use-refcount-on-struct-l2cap_chan-co.patch"
- [14]: <patches/linux/0004-Bluetooth-L2CAP-free-the-leaking-struct-l2cap_conn.patch>
"0004-Bluetooth-L2CAP-free-the-leaking-struct-l2cap_conn.patch"
- [15]: <patches/linux/0005-Bluetooth-introduce-hci_conn_free-for-better-structu.patch>
"0005-Bluetooth-introduce-hci_conn_free-for-better-structu.patch"
- [16]: <patches/linux/0006-Bluetooth-L2CAP-inc-refcount-if-reuse-struct-l2cap_c.patch>
"0006-Bluetooth-L2CAP-inc-refcount-if-reuse-struct-l2cap_c.patch"
- [17]: <patches/linux/0007-Bluetooth-unlink-objects-to-avoid-use-after-free.patch>
"0007-Bluetooth-unlink-objects-to-avoid-use-after-free.patch"
- [18]: <https://gitlab.com/essensium-mind/ble-memleak-repro/-/blob/main/package/ble-memleak-repro/ble-memleak-repro.c?ref_type=heads>
"ble-memleak-repro.c"
- [19]: <https://lore.kernel.org/linux-bluetooth/0000000000000fd01206046e7410@xxxxxxxxxx/T/#u>
"[syzbot] [bluetooth?] memory leak in hci_conn_add (2)"
2023-09-02 23:25:00 -0700
---
Christophe Roullier (1):
ARM: dts: stm32: Add Bluetooth (usart2) feature on stm32mp157x
Olivier L'Heureux (6):
Bluetooth: add many traces for allocation/free/refcounting
Bluetooth: L2CAP: use refcount on struct l2cap_chan->conn
Bluetooth: L2CAP: free the leaking struct l2cap_conn
Bluetooth: introduce hci_conn_free() for better structure
Bluetooth: L2CAP: inc refcount if reuse struct l2cap_conn
Bluetooth: unlink objects to avoid use-after-free
arch/arm/boot/dts/st/stm32mp157c-dk2.dts | 11 ++++++-
include/net/bluetooth/hci_core.h | 7 +++--
net/bluetooth/hci_conn.c | 18 ++++++++++++
net/bluetooth/hci_sysfs.c | 8 ++++-
net/bluetooth/l2cap_core.c | 37 ++++++++++++++++++++----
5 files changed, 72 insertions(+), 9 deletions(-)
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
--
2.39.2
