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