Hi,
My fuzzer found two null pointer exceptions in aosp_do_open and msft_do_open, both triggered by the
same frame sequence on the current bluetooth-next master.
BUG: kernel NULL pointer dereference, address: 0000000000000070
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 41 Comm: kworker/u3:0 Not tainted 5.18.0-rc7-00850-g33b44d809538 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Workqueue: hci0 hci_power_on
RIP: 0010:aosp_do_open+0x7e/0x230
Call Trace:
<TASK>
hci_dev_open_sync+0x98b/0x1060
hci_power_on+0x84/0x350
process_one_work+0x2a6/0x5d0
worker_thread+0x4a/0x3d0
? process_one_work+0x5d0/0x5d0
kthread+0xed/0x120
? kthread_complete_and_exit+0x20/0x20
ret_from_fork+0x22/0x30
</TASK>
The null pointer deref occurs in the skb length check (net/bluetooth/aosp.c:64), the skb returned
previously by __hci_cmd_sync is null:
/* LE Get Vendor Capabilities Command */
skb = __hci_cmd_sync(hdev, hci_opcode_pack(0x3f, 0x153), 0, NULL,
HCI_CMD_TIMEOUT);
if (IS_ERR(skb)) {
bt_dev_err(hdev, "AOSP get vendor capabilities (%ld)",
PTR_ERR(skb));
return;
}
/* A basic length check */
if (skb->len < VENDOR_CAPA_BASE_SIZE)
Bisected to: [d0b137062b2de75b264b84143d21c98abc5f5ad2] Bluetooth: hci_sync: Rework init stages
It occurs when an AOSP resp. MSFT capable controller sends the following two different
Command Status (0x0f) for Reset (0x0c03) on initialization:
Status Octet: Unknown HCI Command (0x01)
0000 04 0f 04 01 02 03 0c
Status Octet: Pending (0x00)
0000 04 0f 04 00 02 03 0c
The problem seems to be that __hci_cmd_sync returns null, and not an PTR_ERR. The affected code
for msft_do_open is similar (msft.c:128ff):
skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp,
HCI_CMD_TIMEOUT);
if (IS_ERR(skb)) {
bt_dev_err(hdev, "Failed to read MSFT supported features (%ld)",
PTR_ERR(skb));
return false;
}
if (skb->len < sizeof(*rp)) {
Which one is triggered depends on the device reporting itself as either AOSP or MSFT capable.
If it indicates both, it depends on timing.
I think that can only be triggered by a malicious or broken Bluetooth controller, but there
might be an underlying issue with __hci_cmd_sync returning null instead of an ERR_PTR.
Best
Sönke
Hi Sönke,
On Thu, Jul 21, 2022 at 3:16 AM Sönke Huster <[email protected]> wrote:
>
> Hi,
>
> My fuzzer found two null pointer exceptions in aosp_do_open and msft_do_open, both triggered by the
> same frame sequence on the current bluetooth-next master.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000070
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 0 PID: 41 Comm: kworker/u3:0 Not tainted 5.18.0-rc7-00850-g33b44d809538 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
> Workqueue: hci0 hci_power_on
> RIP: 0010:aosp_do_open+0x7e/0x230
> Call Trace:
> <TASK>
> hci_dev_open_sync+0x98b/0x1060
> hci_power_on+0x84/0x350
> process_one_work+0x2a6/0x5d0
> worker_thread+0x4a/0x3d0
> ? process_one_work+0x5d0/0x5d0
> kthread+0xed/0x120
> ? kthread_complete_and_exit+0x20/0x20
> ret_from_fork+0x22/0x30
> </TASK>
>
> The null pointer deref occurs in the skb length check (net/bluetooth/aosp.c:64), the skb returned
> previously by __hci_cmd_sync is null:
>
> /* LE Get Vendor Capabilities Command */
> skb = __hci_cmd_sync(hdev, hci_opcode_pack(0x3f, 0x153), 0, NULL,
> HCI_CMD_TIMEOUT);
> if (IS_ERR(skb)) {
> bt_dev_err(hdev, "AOSP get vendor capabilities (%ld)",
> PTR_ERR(skb));
> return;
> }
>
> /* A basic length check */
> if (skb->len < VENDOR_CAPA_BASE_SIZE)
>
> Bisected to: [d0b137062b2de75b264b84143d21c98abc5f5ad2] Bluetooth: hci_sync: Rework init stages
>
> It occurs when an AOSP resp. MSFT capable controller sends the following two different
> Command Status (0x0f) for Reset (0x0c03) on initialization:
>
> Status Octet: Unknown HCI Command (0x01)
> 0000 04 0f 04 01 02 03 0c
>
> Status Octet: Pending (0x00)
> 0000 04 0f 04 00 02 03 0c
>
> The problem seems to be that __hci_cmd_sync returns null, and not an PTR_ERR. The affected code
> for msft_do_open is similar (msft.c:128ff):
>
> skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp,
> HCI_CMD_TIMEOUT);
> if (IS_ERR(skb)) {
> bt_dev_err(hdev, "Failed to read MSFT supported features (%ld)",
> PTR_ERR(skb));
> return false;
> }
>
> if (skb->len < sizeof(*rp)) {
>
> Which one is triggered depends on the device reporting itself as either AOSP or MSFT capable.
> If it indicates both, it depends on timing.
>
> I think that can only be triggered by a malicious or broken Bluetooth controller, but there
> might be an underlying issue with __hci_cmd_sync returning null instead of an ERR_PTR.
Yep that is most likely a Command Status being returned which isn't
expected for the commands above:
/* If command return a status event skb will be set to NULL as there are
* no parameters, in case of failure IS_ERR(skb) would have be set to
* the actual error would be found with PTR_ERR(skb).
*/
if (!skb)
return 0;
So perhaps we should convert these checks to IS_ERR_OR_NULL.
> Best
> Sönke
--
Luiz Augusto von Dentz
Hi Luiz,
On 21.07.22 19:52, Luiz Augusto von Dentz wrote:
> Hi Sönke,
>
> On Thu, Jul 21, 2022 at 3:16 AM Sönke Huster <[email protected]> wrote:
>>
>> Hi,
>>
>> My fuzzer found two null pointer exceptions in aosp_do_open and msft_do_open, both triggered by the
>> same frame sequence on the current bluetooth-next master.
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000070
>> #PF: supervisor read access in kernel mode
>> #PF: error_code(0x0000) - not-present page
>> PGD 0 P4D 0
>> Oops: 0000 [#1] PREEMPT SMP NOPTI
>> CPU: 0 PID: 41 Comm: kworker/u3:0 Not tainted 5.18.0-rc7-00850-g33b44d809538 #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>> Workqueue: hci0 hci_power_on
>> RIP: 0010:aosp_do_open+0x7e/0x230
>> Call Trace:
>> <TASK>
>> hci_dev_open_sync+0x98b/0x1060
>> hci_power_on+0x84/0x350
>> process_one_work+0x2a6/0x5d0
>> worker_thread+0x4a/0x3d0
>> ? process_one_work+0x5d0/0x5d0
>> kthread+0xed/0x120
>> ? kthread_complete_and_exit+0x20/0x20
>> ret_from_fork+0x22/0x30
>> </TASK>
>>
>> The null pointer deref occurs in the skb length check (net/bluetooth/aosp.c:64), the skb returned
>> previously by __hci_cmd_sync is null:
>>
>> /* LE Get Vendor Capabilities Command */
>> skb = __hci_cmd_sync(hdev, hci_opcode_pack(0x3f, 0x153), 0, NULL,
>> HCI_CMD_TIMEOUT);
>> if (IS_ERR(skb)) {
>> bt_dev_err(hdev, "AOSP get vendor capabilities (%ld)",
>> PTR_ERR(skb));
>> return;
>> }
>>
>> /* A basic length check */
>> if (skb->len < VENDOR_CAPA_BASE_SIZE)
>>
>> Bisected to: [d0b137062b2de75b264b84143d21c98abc5f5ad2] Bluetooth: hci_sync: Rework init stages
>>
>> It occurs when an AOSP resp. MSFT capable controller sends the following two different
>> Command Status (0x0f) for Reset (0x0c03) on initialization:
>>
>> Status Octet: Unknown HCI Command (0x01)
>> 0000 04 0f 04 01 02 03 0c
>>
>> Status Octet: Pending (0x00)
>> 0000 04 0f 04 00 02 03 0c
>>
>> The problem seems to be that __hci_cmd_sync returns null, and not an PTR_ERR. The affected code
>> for msft_do_open is similar (msft.c:128ff):
>>
>> skb = __hci_cmd_sync(hdev, hdev->msft_opcode, sizeof(cp), &cp,
>> HCI_CMD_TIMEOUT);
>> if (IS_ERR(skb)) {
>> bt_dev_err(hdev, "Failed to read MSFT supported features (%ld)",
>> PTR_ERR(skb));
>> return false;
>> }
>>
>> if (skb->len < sizeof(*rp)) {
>>
>> Which one is triggered depends on the device reporting itself as either AOSP or MSFT capable.
>> If it indicates both, it depends on timing.
>>
>> I think that can only be triggered by a malicious or broken Bluetooth controller, but there
>> might be an underlying issue with __hci_cmd_sync returning null instead of an ERR_PTR.
>
> Yep that is most likely a Command Status being returned which isn't
> expected for the commands above:
>
> /* If command return a status event skb will be set to NULL as there are
> * no parameters, in case of failure IS_ERR(skb) would have be set to
> * the actual error would be found with PTR_ERR(skb).
> */
> if (!skb)
> return 0;
>
Sorry, I could not reconstruct the contents of this comment from the code and did not see that comment.
Thus, I thought there is an underlying error in the command response handling.
> So perhaps we should convert these checks to IS_ERR_OR_NULL.
OK, I'll submit a patch today.
>
>> Best
>> Sönke
>
>
>