* [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
@ 2025-04-15 10:27 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-04-15 10:27 UTC (permalink / raw)
To: Wesley Cheng; +Cc: linux-sound
Hello Wesley Cheng,
Commit 326bbc348298 ("ALSA: usb-audio: qcom: Introduce QC USB SND
offloading support") from Apr 9, 2025 (linux-next), leads to the
following Smatch static checker warning:
sound/usb/qcom/qc_audio_offload.c:1364 prepare_qmi_response()
warn: missing error code here? 'snd_soc_usb_find_priv_data()' failed. 'ret' = '0'
sound/usb/qcom/qc_audio_offload.c
1335 static int prepare_qmi_response(struct snd_usb_substream *subs,
1336 struct qmi_uaudio_stream_req_msg_v01 *req_msg,
1337 struct qmi_uaudio_stream_resp_msg_v01 *resp,
1338 int info_idx)
1339 {
1340 struct q6usb_offload *data;
1341 int pcm_dev_num;
1342 int card_num;
1343 u8 *xfer_buf = NULL;
1344 int ret;
1345
1346 pcm_dev_num = (req_msg->usb_token & QMI_STREAM_REQ_DEV_NUM_MASK) >> 8;
1347 card_num = (req_msg->usb_token & QMI_STREAM_REQ_CARD_NUM_MASK) >> 16;
1348
1349 if (!uadev[card_num].ctrl_intf) {
1350 dev_err(&subs->dev->dev, "audio ctrl intf info not cached\n");
1351 ret = -ENODEV;
1352 goto err;
1353 }
1354
1355 ret = uaudio_populate_uac_desc(subs, resp);
1356 if (ret < 0)
1357 goto err;
1358
1359 resp->slot_id = subs->dev->slot_id;
1360 resp->slot_id_valid = 1;
1361
1362 data = snd_soc_usb_find_priv_data(uaudio_qdev->auxdev->dev.parent);
1363 if (!data)
--> 1364 goto err;
error code? It's probably better to return directly because it often
helps avoid "forgot set the error code" bugs. Also it's more readable.
1365
1366 uaudio_qdev->data = data;
1367
1368 resp->std_as_opr_intf_desc_valid = 1;
1369 ret = uaudio_endpoint_setup(subs, subs->data_endpoint, card_num,
1370 &resp->xhci_mem_info.tr_data,
1371 &resp->std_as_data_ep_desc);
1372 if (ret < 0)
1373 goto err;
1374
1375 resp->std_as_data_ep_desc_valid = 1;
1376
1377 if (subs->sync_endpoint) {
1378 ret = uaudio_endpoint_setup(subs, subs->sync_endpoint, card_num,
1379 &resp->xhci_mem_info.tr_sync,
1380 &resp->std_as_sync_ep_desc);
1381 if (ret < 0)
1382 goto drop_data_ep;
1383
1384 resp->std_as_sync_ep_desc_valid = 1;
1385 }
1386
1387 resp->interrupter_num_valid = 1;
1388 resp->controller_num_valid = 0;
1389 ret = usb_get_controller_id(subs->dev);
1390 if (ret >= 0) {
1391 resp->controller_num = ret;
1392 resp->controller_num_valid = 1;
1393 }
1394
1395 /* event ring */
1396 ret = uaudio_event_ring_setup(subs, card_num,
1397 &resp->xhci_mem_info.evt_ring);
1398 if (ret < 0)
1399 goto drop_sync_ep;
1400
1401 uaudio_qdev->er_mapped = true;
1402 resp->interrupter_num = xhci_sideband_interrupter_id(uadev[card_num].sb);
1403
1404 resp->speed_info = get_speed_info(subs->dev->speed);
1405 if (resp->speed_info == USB_QMI_DEVICE_SPEED_INVALID_V01) {
1406 ret = -ENODEV;
1407 goto free_sec_ring;
1408 }
1409
1410 resp->speed_info_valid = 1;
1411
1412 ret = uaudio_transfer_buffer_setup(subs, xfer_buf, req_msg->xfer_buff_size,
1413 &resp->xhci_mem_info.xfer_buff);
1414 if (ret < 0) {
1415 ret = -ENOMEM;
1416 goto free_sec_ring;
1417 }
1418
1419 resp->xhci_mem_info_valid = 1;
1420
1421 if (!atomic_read(&uadev[card_num].in_use)) {
1422 kref_init(&uadev[card_num].kref);
1423 init_waitqueue_head(&uadev[card_num].disconnect_wq);
1424 uadev[card_num].num_intf =
1425 subs->dev->config->desc.bNumInterfaces;
1426 uadev[card_num].info = kcalloc(uadev[card_num].num_intf,
1427 sizeof(struct intf_info),
1428 GFP_KERNEL);
1429 if (!uadev[card_num].info) {
1430 ret = -ENOMEM;
1431 goto unmap_er;
1432 }
1433 uadev[card_num].udev = subs->dev;
1434 atomic_set(&uadev[card_num].in_use, 1);
1435 } else {
1436 kref_get(&uadev[card_num].kref);
1437 }
1438
1439 uadev[card_num].usb_core_id = resp->controller_num;
1440
1441 /* cache intf specific info to use it for unmap and free xfer buf */
1442 uadev[card_num].info[info_idx].data_xfer_ring_va =
1443 IOVA_MASK(resp->xhci_mem_info.tr_data.va);
1444 uadev[card_num].info[info_idx].data_xfer_ring_size = PAGE_SIZE;
1445 uadev[card_num].info[info_idx].sync_xfer_ring_va =
1446 IOVA_MASK(resp->xhci_mem_info.tr_sync.va);
1447 uadev[card_num].info[info_idx].sync_xfer_ring_size = PAGE_SIZE;
1448 uadev[card_num].info[info_idx].xfer_buf_va =
1449 IOVA_MASK(resp->xhci_mem_info.xfer_buff.va);
1450 uadev[card_num].info[info_idx].xfer_buf_pa =
1451 resp->xhci_mem_info.xfer_buff.pa;
1452 uadev[card_num].info[info_idx].xfer_buf_size =
1453 resp->xhci_mem_info.xfer_buff.size;
1454 uadev[card_num].info[info_idx].data_ep_pipe = subs->data_endpoint ?
1455 subs->data_endpoint->pipe : 0;
1456 uadev[card_num].info[info_idx].sync_ep_pipe = subs->sync_endpoint ?
1457 subs->sync_endpoint->pipe : 0;
1458 uadev[card_num].info[info_idx].data_ep_idx = subs->data_endpoint ?
1459 subs->data_endpoint->ep_num : 0;
1460 uadev[card_num].info[info_idx].sync_ep_idx = subs->sync_endpoint ?
1461 subs->sync_endpoint->ep_num : 0;
1462 uadev[card_num].info[info_idx].xfer_buf = xfer_buf;
1463 uadev[card_num].info[info_idx].pcm_card_num = card_num;
1464 uadev[card_num].info[info_idx].pcm_dev_num = pcm_dev_num;
1465 uadev[card_num].info[info_idx].direction = subs->direction;
1466 uadev[card_num].info[info_idx].intf_num = subs->cur_audiofmt->iface;
1467 uadev[card_num].info[info_idx].in_use = true;
1468
1469 set_bit(card_num, &uaudio_qdev->card_slot);
1470
1471 return 0;
1472
1473 unmap_er:
1474 uaudio_iommu_unmap(MEM_EVENT_RING, IOVA_BASE, PAGE_SIZE, PAGE_SIZE);
1475 free_sec_ring:
1476 xhci_sideband_remove_interrupter(uadev[card_num].sb);
1477 drop_sync_ep:
1478 if (subs->sync_endpoint) {
1479 uaudio_iommu_unmap(MEM_XFER_RING,
1480 IOVA_MASK(resp->xhci_mem_info.tr_sync.va),
1481 PAGE_SIZE, PAGE_SIZE);
1482 xhci_sideband_remove_endpoint(uadev[card_num].sb,
1483 usb_pipe_endpoint(subs->dev, subs->sync_endpoint->pipe));
1484 }
1485 drop_data_ep:
1486 uaudio_iommu_unmap(MEM_XFER_RING, IOVA_MASK(resp->xhci_mem_info.tr_data.va),
1487 PAGE_SIZE, PAGE_SIZE);
1488 xhci_sideband_remove_endpoint(uadev[card_num].sb,
1489 usb_pipe_endpoint(subs->dev, subs->data_endpoint->pipe));
1490
1491 err:
1492 return ret;
1493 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
@ 2025-04-15 10:45 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-04-15 10:45 UTC (permalink / raw)
To: Wesley Cheng; +Cc: linux-sound
Hello Wesley Cheng,
This is a semi-automatic email about new static checker warnings.
Commit 326bbc348298 ("ALSA: usb-audio: qcom: Introduce QC USB SND
offloading support") from Apr 9, 2025, leads to the following Smatch
complaint:
sound/usb/qcom/qc_audio_offload.c:1454 prepare_qmi_response()
warn: variable dereferenced before check 'subs->data_endpoint' (see line 1369)
sound/usb/qcom/qc_audio_offload.c
1368 resp->std_as_opr_intf_desc_valid = 1;
1369 ret = uaudio_endpoint_setup(subs, subs->data_endpoint, card_num,
^^^^^^^^^^^^^^^^^^^
this dereferences subs->data_endpoint without checking for NULL
1370 &resp->xhci_mem_info.tr_data,
1371 &resp->std_as_data_ep_desc);
1372 if (ret < 0)
1373 goto err;
1374
1375 resp->std_as_data_ep_desc_valid = 1;
1376
1377 if (subs->sync_endpoint) {
1378 ret = uaudio_endpoint_setup(subs, subs->sync_endpoint, card_num,
1379 &resp->xhci_mem_info.tr_sync,
1380 &resp->std_as_sync_ep_desc);
1381 if (ret < 0)
1382 goto drop_data_ep;
1383
1384 resp->std_as_sync_ep_desc_valid = 1;
1385 }
1386
1387 resp->interrupter_num_valid = 1;
1388 resp->controller_num_valid = 0;
1389 ret = usb_get_controller_id(subs->dev);
1390 if (ret >= 0) {
1391 resp->controller_num = ret;
1392 resp->controller_num_valid = 1;
1393 }
1394
1395 /* event ring */
1396 ret = uaudio_event_ring_setup(subs, card_num,
1397 &resp->xhci_mem_info.evt_ring);
1398 if (ret < 0)
1399 goto drop_sync_ep;
1400
1401 uaudio_qdev->er_mapped = true;
1402 resp->interrupter_num = xhci_sideband_interrupter_id(uadev[card_num].sb);
1403
1404 resp->speed_info = get_speed_info(subs->dev->speed);
1405 if (resp->speed_info == USB_QMI_DEVICE_SPEED_INVALID_V01) {
1406 ret = -ENODEV;
1407 goto free_sec_ring;
1408 }
1409
1410 resp->speed_info_valid = 1;
1411
1412 ret = uaudio_transfer_buffer_setup(subs, xfer_buf, req_msg->xfer_buff_size,
1413 &resp->xhci_mem_info.xfer_buff);
1414 if (ret < 0) {
1415 ret = -ENOMEM;
1416 goto free_sec_ring;
1417 }
1418
1419 resp->xhci_mem_info_valid = 1;
1420
1421 if (!atomic_read(&uadev[card_num].in_use)) {
1422 kref_init(&uadev[card_num].kref);
1423 init_waitqueue_head(&uadev[card_num].disconnect_wq);
1424 uadev[card_num].num_intf =
1425 subs->dev->config->desc.bNumInterfaces;
1426 uadev[card_num].info = kcalloc(uadev[card_num].num_intf,
1427 sizeof(struct intf_info),
1428 GFP_KERNEL);
1429 if (!uadev[card_num].info) {
1430 ret = -ENOMEM;
1431 goto unmap_er;
1432 }
1433 uadev[card_num].udev = subs->dev;
1434 atomic_set(&uadev[card_num].in_use, 1);
1435 } else {
1436 kref_get(&uadev[card_num].kref);
1437 }
1438
1439 uadev[card_num].usb_core_id = resp->controller_num;
1440
1441 /* cache intf specific info to use it for unmap and free xfer buf */
1442 uadev[card_num].info[info_idx].data_xfer_ring_va =
1443 IOVA_MASK(resp->xhci_mem_info.tr_data.va);
1444 uadev[card_num].info[info_idx].data_xfer_ring_size = PAGE_SIZE;
1445 uadev[card_num].info[info_idx].sync_xfer_ring_va =
1446 IOVA_MASK(resp->xhci_mem_info.tr_sync.va);
1447 uadev[card_num].info[info_idx].sync_xfer_ring_size = PAGE_SIZE;
1448 uadev[card_num].info[info_idx].xfer_buf_va =
1449 IOVA_MASK(resp->xhci_mem_info.xfer_buff.va);
1450 uadev[card_num].info[info_idx].xfer_buf_pa =
1451 resp->xhci_mem_info.xfer_buff.pa;
1452 uadev[card_num].info[info_idx].xfer_buf_size =
1453 resp->xhci_mem_info.xfer_buff.size;
1454 uadev[card_num].info[info_idx].data_ep_pipe = subs->data_endpoint ?
^^^^^^^^^^^^^^^^^^^
So this check is too late
1455 subs->data_endpoint->pipe : 0;
1456 uadev[card_num].info[info_idx].sync_ep_pipe = subs->sync_endpoint ?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
@ 2025-04-15 10:46 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-04-15 10:46 UTC (permalink / raw)
To: Wesley Cheng; +Cc: linux-sound
Hello Wesley Cheng,
Commit 326bbc348298 ("ALSA: usb-audio: qcom: Introduce QC USB SND
offloading support") from Apr 9, 2025 (linux-next), leads to the
following Smatch static checker warning:
sound/usb/qcom/qc_audio_offload.c:762 qmi_stop_session()
error: we previously assumed 'subs' could be null (see line 761)
sound/usb/qcom/qc_audio_offload.c
737 static void qmi_stop_session(void)
738 {
739 struct snd_usb_substream *subs;
740 struct usb_host_endpoint *ep;
741 struct snd_usb_audio *chip;
742 struct intf_info *info;
743 int pcm_card_num;
744 int if_idx;
745 int idx;
746
747 mutex_lock(&qdev_mutex);
748 /* find all active intf for set alt 0 and cleanup usb audio dev */
749 for (idx = 0; idx < SNDRV_CARDS; idx++) {
750 if (!atomic_read(&uadev[idx].in_use))
751 continue;
752
753 chip = uadev[idx].chip;
754 for (if_idx = 0; if_idx < uadev[idx].num_intf; if_idx++) {
755 if (!uadev[idx].info || !uadev[idx].info[if_idx].in_use)
756 continue;
757 info = &uadev[idx].info[if_idx];
758 pcm_card_num = info->pcm_card_num;
759 subs = find_substream(pcm_card_num, info->pcm_dev_num,
760 info->direction);
761 if (!subs || !chip || atomic_read(&chip->shutdown)) {
^^^^
Check for NULL
--> 762 dev_err(&subs->dev->dev,
^^^^^^^^^^^^^^^
Unchecked derefrence
763 "no sub for c#%u dev#%u dir%u\n",
764 info->pcm_card_num,
765 info->pcm_dev_num,
766 info->direction);
767 continue;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
@ 2025-04-15 10:46 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-04-15 10:46 UTC (permalink / raw)
To: Wesley Cheng; +Cc: linux-sound
Hello Wesley Cheng,
Commit 326bbc348298 ("ALSA: usb-audio: qcom: Introduce QC USB SND
offloading support") from Apr 9, 2025 (linux-next), leads to the
following Smatch static checker warning:
sound/usb/qcom/qc_audio_offload.c:773 qmi_stop_session()
error: uninitialized symbol 'ep'.
sound/usb/qcom/qc_audio_offload.c
737 static void qmi_stop_session(void)
738 {
739 struct snd_usb_substream *subs;
740 struct usb_host_endpoint *ep;
741 struct snd_usb_audio *chip;
742 struct intf_info *info;
743 int pcm_card_num;
744 int if_idx;
745 int idx;
746
747 mutex_lock(&qdev_mutex);
748 /* find all active intf for set alt 0 and cleanup usb audio dev */
749 for (idx = 0; idx < SNDRV_CARDS; idx++) {
750 if (!atomic_read(&uadev[idx].in_use))
751 continue;
752
753 chip = uadev[idx].chip;
754 for (if_idx = 0; if_idx < uadev[idx].num_intf; if_idx++) {
755 if (!uadev[idx].info || !uadev[idx].info[if_idx].in_use)
756 continue;
757 info = &uadev[idx].info[if_idx];
758 pcm_card_num = info->pcm_card_num;
759 subs = find_substream(pcm_card_num, info->pcm_dev_num,
760 info->direction);
761 if (!subs || !chip || atomic_read(&chip->shutdown)) {
762 dev_err(&subs->dev->dev,
763 "no sub for c#%u dev#%u dir%u\n",
764 info->pcm_card_num,
765 info->pcm_dev_num,
766 info->direction);
767 continue;
768 }
769 /* Release XHCI endpoints */
770 if (info->data_ep_pipe)
771 ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
772 info->data_ep_pipe);
^^
ep not set on else path.
--> 773 xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb, ep);
^^
uninitialized.
774
775 if (info->sync_ep_pipe)
776 ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
777 info->sync_ep_pipe);
778 xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb, ep);
same
779
780 disable_audio_stream(subs);
781 }
782 atomic_set(&uadev[idx].in_use, 0);
783 mutex_lock(&chip->mutex);
784 uaudio_dev_cleanup(&uadev[idx]);
785 mutex_unlock(&chip->mutex);
786 }
787 mutex_unlock(&qdev_mutex);
788 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support
@ 2025-04-15 10:47 Dan Carpenter
0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-04-15 10:47 UTC (permalink / raw)
To: Wesley Cheng; +Cc: linux-sound
Hello Wesley Cheng,
Commit 326bbc348298 ("ALSA: usb-audio: qcom: Introduce QC USB SND
offloading support") from Apr 9, 2025 (linux-next), leads to the
following Smatch static checker warning:
sound/usb/qcom/qc_audio_offload.c:785 qmi_stop_session()
error: we previously assumed 'chip' could be null (see line 761)
sound/usb/qcom/qc_audio_offload.c
737 static void qmi_stop_session(void)
738 {
739 struct snd_usb_substream *subs;
740 struct usb_host_endpoint *ep;
741 struct snd_usb_audio *chip;
742 struct intf_info *info;
743 int pcm_card_num;
744 int if_idx;
745 int idx;
746
747 mutex_lock(&qdev_mutex);
748 /* find all active intf for set alt 0 and cleanup usb audio dev */
749 for (idx = 0; idx < SNDRV_CARDS; idx++) {
750 if (!atomic_read(&uadev[idx].in_use))
751 continue;
752
753 chip = uadev[idx].chip;
We should check if chip is NULL here
754 for (if_idx = 0; if_idx < uadev[idx].num_intf; if_idx++) {
755 if (!uadev[idx].info || !uadev[idx].info[if_idx].in_use)
756 continue;
757 info = &uadev[idx].info[if_idx];
758 pcm_card_num = info->pcm_card_num;
759 subs = find_substream(pcm_card_num, info->pcm_dev_num,
760 info->direction);
761 if (!subs || !chip || atomic_read(&chip->shutdown)) {
^^^^
Instead of here.
762 dev_err(&subs->dev->dev,
763 "no sub for c#%u dev#%u dir%u\n",
764 info->pcm_card_num,
765 info->pcm_dev_num,
766 info->direction);
767 continue;
768 }
769 /* Release XHCI endpoints */
770 if (info->data_ep_pipe)
771 ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
772 info->data_ep_pipe);
773 xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb, ep);
774
775 if (info->sync_ep_pipe)
776 ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
777 info->sync_ep_pipe);
778 xhci_sideband_remove_endpoint(uadev[pcm_card_num].sb, ep);
779
780 disable_audio_stream(subs);
781 }
782 atomic_set(&uadev[idx].in_use, 0);
783 mutex_lock(&chip->mutex);
^^^^^
If it is NULL this will crash.
784 uaudio_dev_cleanup(&uadev[idx]);
--> 785 mutex_unlock(&chip->mutex);
786 }
787 mutex_unlock(&qdev_mutex);
788 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-15 10:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-15 10:27 [bug report] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2025-04-15 10:45 Dan Carpenter
2025-04-15 10:46 Dan Carpenter
2025-04-15 10:46 Dan Carpenter
2025-04-15 10:47 Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox