[PATCH 0/2] ALSA: usb-audio: qcom: fix QMI stream-request handling

[PATCH 0/2] ALSA: usb-audio: qcom: fix QMI stream-request handling

[Michael Bommarito]

Two fixes in handle_uaudio_stream_req(), the QMI handler for the Qualcomm
USB audio offload stream enable/disable requests (reachable from
unprivileged local userspace over AF_QIPCRTR):

Patch 1: the disable path dereferences uadev[card].info[info_idx] without
the info_idx >= 0 guard the enable path and the cleanup label have.
info_idx is -EINVAL on a non-match and .info is allocated only on enable,
so this is not only a NULL deref: when .info is allocated (card enabled
once) and the disable names a non-matching interface, &info[-EINVAL]
points before the allocation and the pipe fields are an out-of-bounds slab
read plus a conditional out-of-bounds 4-byte zero-write. A never-enabled
card instead faults on a wild pointer (oops).

Patch 2: on enable, subs->opened is set before the service_interval is
validated; an invalid interval jumps out without clearing it, wedging the
substream at -EBUSY until disable/disconnect.

Impact: on an affected Qualcomm platform, a local unprivileged process that
can drive the QMI disable path for a card with no active interface would
oops the kernel (patch 1); patch 2 leaves a substream wedged at -EBUSY.
See the
confidence note below: this is a static finding, not yet reproduced.

Confidence and testing: this series is from static analysis of current
mainline; I have NOT reproduced it. CONFIG_SND_USB_AUDIO_QMI builds only on
Qualcomm SoCs with an audio DSP (no x86 build and no Qualcomm hardware
here), so there is no splat to show. What I did verify by reading current
torvalds/master (the offload driver was mainlined in 6.16):

 * Patch 1: in handle_uaudio_stream_req() the enable branch checks
   "info_idx < 0" before use and the response: cleanup label checks
   "info_idx >= 0", but the disable branch between them dereferences
   uadev[pcm_card_num].info[info_idx] with no such check. info_idx is the
   negative return of info_idx_from_ifnum() when no interface matches, and
   .info is a pointer allocated only on enable, so for a
   connected-but-never-enabled card the disable branch forms and then
   dereferences a wild pointer.

 * Patch 2: subs->opened is set in the enable branch before the
   service_interval validation; that validation can "goto response" without
   clearing it, and the response label only clears opened on the disable
   side, so the substream stays wedged.

What I am NOT certain of, and would ask you to confirm on a Qualcomm build:
whether the QMI flow actually lets a disable request reach the disable
branch with info_idx < 0 (a disable for a card with no matching interface).
If the userspace client cannot produce that ordering, patch 1 is hardening
rather than a live oops; patch 2 stands either way. Both patches only add
guards that already exist elsewhere in the same function, so they are low
risk to apply. Please also confirm the Fixes: tag against your tree.

Michael Bommarito (2):
  ALSA: usb-audio: qcom: reject stream disable with no active interface
  ALSA: usb-audio: qcom: clear opened when stream enable fails

 sound/usb/qcom/qc_audio_offload.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH 1/2] ALSA: usb-audio: qcom: reject stream disable with no active interface

[Michael Bommarito]

handle_uaudio_stream_req() resolves an interface index with
info_idx_from_ifnum(), which returns -EINVAL when no interface matches.
The enable branch and the response: cleanup label both guard against a
negative index, but the disable branch does not: it forms
info = &uadev[pcm_card_num].info[info_idx] and dereferences it.

uadev[].info is a pointer allocated only when a stream is first enabled,
so a negative info_idx on the disable path is unsafe in two ways:

 - If the card was never enabled, .info is NULL and &info[-EINVAL] is a
   wild pointer; reading info->data_ep_pipe faults (kernel oops).

 - If the card was enabled at least once (.info allocated) and the
   disable names an interface that does not match, &info[-EINVAL] points
   before the allocation; info->data_ep_pipe / info->sync_ep_pipe are an
   out-of-bounds slab read and, when non-zero, an out-of-bounds 4-byte
   write (both pipe fields are cleared to 0). That is memory corruption,
   not just a NULL dereference.

The request is reachable from unprivileged local userspace over
AF_QIPCRTR. Reject a disable request with no resolved interface, matching
the guard the enable path already has.

Fixes: 326bbc348298a ("ALSA: usb-audio: qcom: Introduce QC USB SND offloading support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Not reproduced: static analysis against current mainline only (no Qualcomm
hardware; CONFIG_SND_USB_AUDIO_QMI does not build on x86). The enable
branch and the response: cleanup label already guard info_idx; this adds
the same guard to the disable branch. Please confirm on a Qualcomm build
whether a disable request can reach this branch with info_idx < 0, and
whether the out-of-bounds write in the .info-allocated case is reachable.

 sound/usb/qcom/qc_audio_offload.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index a3f90cc7c6cad..852a91e4b8686 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -1640,6 +1640,11 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 			subs->opened = 0;
 		}
 	} else {
+		if (info_idx < 0) {
+			ret = -EINVAL;
+			goto response;
+		}
+
 		info = &uadev[pcm_card_num].info[info_idx];
 		if (info->data_ep_pipe) {
 			ep = usb_pipe_endpoint(uadev[pcm_card_num].udev,
-- 
2.53.0

[PATCH 2/2] ALSA: usb-audio: qcom: clear opened when stream enable fails

[Michael Bommarito]

On enable, subs->opened is set before the service_interval is validated;
an invalid interval jumps to the response label without clearing it, so
the substream is wedged at -EBUSY until a disable or disconnect.

Clear subs->opened on the enable error path.

Fixes: 326bbc348298a ("ALSA: usb-audio: qcom: Introduce QC USB SND offloading support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 sound/usb/qcom/qc_audio_offload.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index 852a91e4b8686..221f87791bc94 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -1618,8 +1618,13 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
 	if (req_msg->service_interval_valid) {
 		ret = get_data_interval_from_si(subs,
 						req_msg->service_interval);
-		if (ret == -EINVAL)
+		if (ret == -EINVAL) {
+			if (req_msg->enable) {
+				guard(mutex)(&chip->mutex);
+				subs->opened = 0;
+			}
 			goto response;
+		}
 
 		datainterval = ret;
 	}
-- 
2.53.0

Re: [PATCH 0/2] ALSA: usb-audio: qcom: fix QMI stream-request handling

[Takashi Iwai]

On Thu, 18 Jun 2026 04:51:24 +0200,
Michael Bommarito wrote:
> 
> Two fixes in handle_uaudio_stream_req(), the QMI handler for the Qualcomm
> USB audio offload stream enable/disable requests (reachable from
> unprivileged local userspace over AF_QIPCRTR):
> 
> Patch 1: the disable path dereferences uadev[card].info[info_idx] without
> the info_idx >= 0 guard the enable path and the cleanup label have.
> info_idx is -EINVAL on a non-match and .info is allocated only on enable,
> so this is not only a NULL deref: when .info is allocated (card enabled
> once) and the disable names a non-matching interface, &info[-EINVAL]
> points before the allocation and the pipe fields are an out-of-bounds slab
> read plus a conditional out-of-bounds 4-byte zero-write. A never-enabled
> card instead faults on a wild pointer (oops).
> 
> Patch 2: on enable, subs->opened is set before the service_interval is
> validated; an invalid interval jumps out without clearing it, wedging the
> substream at -EBUSY until disable/disconnect.
> 
> Impact: on an affected Qualcomm platform, a local unprivileged process that
> can drive the QMI disable path for a card with no active interface would
> oops the kernel (patch 1); patch 2 leaves a substream wedged at -EBUSY.
> See the
> confidence note below: this is a static finding, not yet reproduced.
> 
> Confidence and testing: this series is from static analysis of current
> mainline; I have NOT reproduced it. CONFIG_SND_USB_AUDIO_QMI builds only on
> Qualcomm SoCs with an audio DSP (no x86 build and no Qualcomm hardware
> here), so there is no splat to show. What I did verify by reading current
> torvalds/master (the offload driver was mainlined in 6.16):
> 
>  * Patch 1: in handle_uaudio_stream_req() the enable branch checks
>    "info_idx < 0" before use and the response: cleanup label checks
>    "info_idx >= 0", but the disable branch between them dereferences
>    uadev[pcm_card_num].info[info_idx] with no such check. info_idx is the
>    negative return of info_idx_from_ifnum() when no interface matches, and
>    .info is a pointer allocated only on enable, so for a
>    connected-but-never-enabled card the disable branch forms and then
>    dereferences a wild pointer.
> 
>  * Patch 2: subs->opened is set in the enable branch before the
>    service_interval validation; that validation can "goto response" without
>    clearing it, and the response label only clears opened on the disable
>    side, so the substream stays wedged.
> 
> What I am NOT certain of, and would ask you to confirm on a Qualcomm build:
> whether the QMI flow actually lets a disable request reach the disable
> branch with info_idx < 0 (a disable for a card with no matching interface).
> If the userspace client cannot produce that ordering, patch 1 is hardening
> rather than a live oops; patch 2 stands either way. Both patches only add
> guards that already exist elsewhere in the same function, so they are low
> risk to apply. Please also confirm the Fixes: tag against your tree.
> 
> Michael Bommarito (2):
>   ALSA: usb-audio: qcom: reject stream disable with no active interface
>   ALSA: usb-audio: qcom: clear opened when stream enable fails

Applied both patches now.  Thanks.


Takashi