[SRU][N][PATCH 0/1] Bluetooth fails to initialize due to a kernel NULL pointer error

[AceLan Kao]

From: "Chia-Lin Kao (AceLan)" <acelan.kao at canonical.com>

BugLink: https://bugs.launchpad.net/bugs/2165873

[Impact]
Bluetooth fails to start on HP systems with a MediaTek MT7922 controller
(CID: 202401-33402) running 6.8.0-139.139~22.04.1.

The kernel oopses during Bluetooth power-on. The worker thread dies, so
hci0 stays in DOWN INIT and its BD address is all zeros. Bluetooth is
unusable. rfkill unblock and hciconfig do not help.

Error log:

  Bluetooth: hci0: HW/SW Version: 0x008a008a, Build Time: 20231120183620
  BUG: kernel NULL pointer dereference, address: 0000000000000219
  Oops: 0000 [#1] PREEMPT SMP NOPTI
  CPU: 9 PID: 340 Comm: kworker/u33:1 Not tainted 6.8.0-139-generic
  Workqueue: hci0 hci_power_on [bluetooth]
  RIP: 0010:__pm_runtime_resume+0x1b/0x80
  Call Trace:
   <TASK>
   usb_autopm_get_interface+0x1d/0x60
   btmtk_usb_hci_wmt_sync+0xb9/0x340 [btmtk]
   btmtk_setup_firmware_79xx+0x1c7/0x360 [btmtk]
   btusb_mtk_setup+0x2d6/0x610 [btusb]
   hci_dev_setup_sync+0x6c/0x440 [bluetooth]
   hci_dev_init_sync+0x3e/0x1c0 [bluetooth]
   hci_dev_open_sync+0x8b/0x350 [bluetooth]
   hci_dev_do_open+0x28/0x70 [bluetooth]
   hci_power_on+0x50/0x210 [bluetooth]

Hits 3 out of 3 boots. 6.8.0-138.138~22.04.1 is fine.

This breaks these checkbox tests:
  com.canonical.certification::bluetooth/detect-output
  com.canonical.certification::bluetooth4/beacon_eddystone_url_hc0

This is not limited to the reporting machine. Any MediaTek MT766x or
MT79xx USB Bluetooth controller takes the same path and hits the same
oops on 6.8.0-139.

[Fix]
6.8.0-139 picked up d019930b0049 ("Bluetooth: btmtk: move
btusb_mtk_hci_wmt_sync to btmtk.c", v6.11) as a stable dependency of the
urb->setup_packet leak fix. That commit moves the WMT command path into
btmtk.c, where it reads intf, udev and ctrl_anchor out of struct
btmtk_data.

The commit that fills those three fields in is the next one in the same
upstream series, and it was not picked:

  5c5e8c52e3ca Bluetooth: btmtk: move btusb_mtk_[setup, shutdown] to btmtk.c (v6.11)

So the pointers stay NULL. The first WMT command calls
usb_autopm_get_interface(NULL) and oopses.

The fix is to backport 5c5e8c52e3ca so the series is complete again. It
adds the missing assignments in btusb_mtk_setup():

  btmtk_data->drv_name = btusb_driver.name;
  btmtk_data->intf = data->intf;
  btmtk_data->udev = data->udev;
  btmtk_data->ctrl_anchor = &data->ctrl_anchor;
  btmtk_data->reset_sync = btusb_mtk_reset;

udev and ctrl_anchor are just as NULL as intf, and are used by the WMT
receive URB, so all of them are needed. A NULL check on intf alone would
only move the oops.

[Test Plan]
On a machine with a MediaTek MT7922 Bluetooth controller.

Boot the machine, then check for the oops:

  $ dmesg | grep -A20 'NULL pointer'
  $ journalctl -b -0 -k | grep btmtk

Check the controller came up:

  $ sudo rfkill unblock bluetooth
  $ hciconfig -a
  $ hcitool dev

Without patch:
  dmesg shows the NULL pointer oops in __pm_runtime_resume with
  btmtk_usb_hci_wmt_sync in the call trace.
  hciconfig shows hci0 DOWN INIT with BD Address 00:00:00:00:00:00.
  hcitool dev lists no device.

With patch:
  no oops in dmesg.
  hciconfig shows hci0 UP RUNNING with a real BD address.
  hcitool dev lists hci0.

Then run the two failing tests:

  $ checkbox-cli run com.canonical.certification::bluetooth/detect-output
  $ checkbox-cli run com.canonical.certification::bluetooth4/beacon_eddystone_url_hc0

Both pass with the patch, both fail without it.

[Where problems could occur]
Could break the btusb and btmtk drivers for all MediaTek Bluetooth
controllers.

This is not a small patch. It moves btusb_mtk_setup() and
btusb_mtk_shutdown() and their helpers out of btusb.c into btmtk.c, so
the whole MediaTek setup path is touched. If the move dropped or changed
something, MediaTek Bluetooth would fail to set up. That would show up as
"Failed to set up firmware" or "Failed to send wmt func ctrl" in dmesg,
or as a timeout during hci0 power-on, and Bluetooth would stay down.

The device reset path also moves (btusb_mtk_subsys_reset becomes
btmtk_usb_subsys_reset). If that is wrong, chip recovery after a firmware
crash would fail and the controller would need a reboot to come back.

Nothing outside drivers/bluetooth is touched. Other Bluetooth vendors
(Intel, Realtek, Qualcomm, Broadcom) are not affected, because btusb only
calls this code for MediaTek devices.

[Other Info]
The patch is upstream in v6.11. It is the second half of a two-commit
series; the first half is already in 6.8.0-139, which is what caused the
regression.

The backport needed manual conflict resolution in drivers/bluetooth/btusb.c.
6.8 is missing the intermediate commits that added fw_flavor handling and
the BTMTK_FIRMWARE_LOADED flag, so the old copies of btusb_mtk_func_query(),
btusb_mtk_uhw_reg_*(), btusb_mtk_reg_read(), btusb_mtk_id_get(),
btusb_mtk_reset_done(), btusb_mtk_subsys_reset() and the open-coded
btusb_mtk_setup() body all conflicted. Those are exactly the functions this
commit deletes, so the upstream side was taken. The resulting
btusb_mtk_setup() is identical to upstream at 5c5e8c52e3ca.

Sean Wang (1):
  Bluetooth: btmtk: move btusb_mtk_[setup, shutdown] to btmtk.c

 drivers/bluetooth/btmtk.c | 454 +++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btmtk.h |  23 +-
 drivers/bluetooth/btusb.c | 379 ++-----------------------------
 3 files changed, 484 insertions(+), 372 deletions(-)

-- 
2.53.0