[PATCH BlueZ v2] adapter: Fix crash on short start discovery reply

[PATCH BlueZ v2] adapter: Fix crash on short start discovery reply

[Iaroslav Voitovych]

start_discovery_complete() validates the reply length before using the
returned parameters:

    if (length < sizeof(*rp)) {
        btd_error(adapter->dev_id,
            "Wrong size of start discovery return parameters");
        discovery_complete(adapter, MGMT_STATUS_FAILED);
        return;
    }

but that check sits below the branch taken when the discovery client
list has emptied while the command was pending, and that branch has
already dereferenced the reply:

    if (!adapter->discovery_list) {
        struct mgmt_cp_stop_discovery cp;

        if (status != MGMT_STATUS_SUCCESS)
            return;

        /* Stop discovering as there are no clients left */
        cp.type = rp->type;      <-- rp may be NULL here

mgmt_cp_start_discovery is a single uint8_t, so rp->type is a one-byte
read at offset 0 and a NULL rp faults immediately.

The mgmt layer can invoke a command callback with no reply parameters:
src/shared/mgmt.c passes length 0 and a NULL param on the Command
Status path:

    case MGMT_EV_CMD_STATUS:
        ...
        DBG(mgmt, "[0x%04x] command 0x%02x status: 0x%02x", ...);
        request_complete(mgmt, cs->status, opcode, index, 0, NULL);

Observed as a crash on a QCA9377 (13d3:3503) whose controller stops
answering HCI during audio use. The archived session logs show exactly
that sequence: the daemon's debug log (bluez 5.72) ends with the
Command Status delivery and the callback entry, for the Start Discovery
it had sent 2.05 s earlier, ten seconds after its last discovery client
had gone; the contemporaneous kernel log records the resulting fault:

  bluetoothd:
  21:03:16.750  discovery_remove() owner :1.125
  21:03:24.943  send_request() [0x0000] command 0x0023
  21:03:26.994  can_read_data() [0x0000] command 0x23 status: 0x00
  21:03:26.994  start_discovery_complete() status 0x00
  kernel:
  21:03:26.995  bluetoothd[2821]: segfault at 0 ...

"command ... status:" is the Command Status branch (Command Complete
logs "complete:"), so the callback ran with status 0, length 0 and a
NULL param, found discovery_list empty, and faulted on rp->type. The
crash site was resolved from the shipped binary:

  bluetoothd[2821]: segfault at 0 ip 00005d6eb1ad1986 \
      sp 00007ffef8e784b0 error 4 in bluetoothd[a6986,5d6eb1a50000+f3000]

  a6986:  41 0f b6 45 00    movzbl 0x0(%r13),%eax  <-- cp.type = rp->type
  a698b:  0f b7 53 04       movzwl 0x4(%rbx),%edx  <-- adapter->dev_id
  a6992:  4c 8d 45 d7       lea    -0x29(%rbp),%r8 <-- &cp

The same delivery, a Command Status with status 0x00 answering a Start
Discovery just sent, was logged five more times on that machine: four
in the crashing daemon's own lifetime and once on a later day. On each
of those the client list was non-empty, so the existing check below
the branch handled it ("Wrong size of start discovery return
parameters"); when the list is empty the branch above is reached
first.

Why the kernel answers Start Discovery with a successful Command
Status rather than the usual one-byte Command Complete is not
established here. The crashing instance arrived while the controller
was timing out HCI commands; the fix does not depend on the cause.

Perform the same length validation before using the reply in the
no-clients branch. The check is added inside that branch rather than
hoisted above it, so that no other path changes behaviour.

Fixes: 3597d1377723 ("adapter: Fix not waiting for start discovery result")
---
v2: quoted code indented with spaces instead of tabs; the quoted fault
    lines re-wrapped to 75 columns. No change to the code.

Context, not part of the change: the crash was recorded on a QCA9377
(13d3:3503) laptop under Ubuntu 24.04 / bluez 5.72-0ubuntu5.5 and the
site was resolved from the stripped distro binary, then checked against
a retained core; the archived session record (daemon debug log and the
kernel log beside it) independently shows the same sequence: client
removed, Start Discovery sent, Command Status 0x00, fault. This guard
has not fired since it was installed; the delivery it guards against
was logged five more times with clients present, so the pre-existing
check caught those. The record and the crash-site method are public:
https://github.com/ivoitovych/qca9377-bt-hang  (patches/bluez/README.md)

 src/adapter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1863,6 +1863,12 @@
 		if (status != MGMT_STATUS_SUCCESS)
 			return;

+		if (length < sizeof(*rp)) {
+			btd_error(adapter->dev_id,
+				"Wrong size of start discovery return parameters");
+			return;
+		}
+
 		/* Stop discovering as there are no clients left */
 		cp.type = rp->type;
 		mgmt_send(adapter->mgmt, MGMT_OP_STOP_DISCOVERY,
--
2.43.0

RE: [BlueZ,v2] adapter: Fix crash on short start discovery reply

[bluez.test.bot]

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

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: src/adapter.c:1863
error: src/adapter.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth