Skip to contentSkip to conversation

Bluetooth: btusb: handle mSBC audio over USB Endpoints

bug report Dan Carpenter 2 participants 2 messages Apr 7, 2020 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

Hello Sathish Narasimman,

This is a semi-automatic email about new static checker warnings.

The patch baac6276c0a9: "Bluetooth: btusb: handle mSBC audio over USB Endpoints" from Apr 3, 2020, leads to the following Smatch complaint:

    drivers/bluetooth/btusb.c:1665 btusb_work()
    error: we previously assumed 'data->isoc' could be null (see line 1642)

drivers/bluetooth/btusb.c
1641 if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {

  1642				err = usb_autopm_get_interface(data->isoc ? data->isoc : data->intf);
                                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This code assumes that "data->isoc" can be NULL.

1643 if (err < 0) {
1644 clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
1645 usb_kill_anchored_urbs(&data->isoc_anchor);
1646 return;
1647 }
1648
1649 set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
1650 }
1651
1652 if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_CVSD) {
1653 if (hdev->voice_setting & 0x0020) {
1654 static const int alts[3] = { 2, 4, 5 };
1655
1656 new_alts = alts[data->sco_num - 1];
1657 } else {
1658 new_alts = data->sco_num;
1659 }
1660 } else if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_TRANSP) {
1661
1662 data->usb_alt6_packet_flow = true;
1663
1664 /* Check if Alt 6 is supported for Transparent audio */
1665 if (btusb_find_altsetting(data, 6))

                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^

The new btusb_find_altsetting() dereferences it without checking.

  1666					new_alts = 6;
  1667				else

regards,
dan carpenter

maintainer
#1 lore raw

Hi Dan,

Quoted 37 lines

This is a semi-automatic email about new static checker warnings.

The patch baac6276c0a9: "Bluetooth: btusb: handle mSBC audio over USB Endpoints" from Apr 3, 2020, leads to the following Smatch complaint:

drivers/bluetooth/btusb.c:1665 btusb_work()
error: we previously assumed 'data->isoc' could be null (see line 1642)

drivers/bluetooth/btusb.c
1641 if (!test_bit(BTUSB_DID_ISO_RESUME, &data->flags)) {

 1642				err = usb_autopm_get_interface(data->isoc ? data->isoc : data->intf);
                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This code assumes that "data->isoc" can be NULL.

1643 if (err < 0) {
1644 clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
1645 usb_kill_anchored_urbs(&data->isoc_anchor);
1646 return;
1647 }
1648
1649 set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
1650 }
1651
1652 if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_CVSD) {
1653 if (hdev->voice_setting & 0x0020) {
1654 static const int alts[3] = { 2, 4, 5 };
1655
1656 new_alts = alts[data->sco_num - 1];
1657 } else {
1658 new_alts = data->sco_num;
1659 }
1660 } else if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_TRANSP) {
1661
1662 data->usb_alt6_packet_flow = true;
1663
1664 /* Check if Alt 6 is supported for Transparent audio */
1665 if (btusb_find_altsetting(data, 6))

Quoted 5 lines
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^

The new btusb_find_altsetting() dereferences it without checking.

1666 new_alts = 6;
1667 else

while it is unlikely that data->isoc is NULL, this can actually happen in some weird cases where the USB device doesn’t provide ISOC endpoints, but still has SCO transport support. So yes, we should be adding a NULL check in btusb_find_altsetting.

Regards

Marcel