[bug report] Bluetooth: btusb: handle mSBC audio over USB Endpoints

[bug report] Bluetooth: btusb: handle mSBC audio over USB Endpoints

[Dan Carpenter]

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

Re: [bug report] Bluetooth: btusb: handle mSBC audio over USB Endpoints

[Marcel Holtmann]

Hi Dan,

> 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

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