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

[Michael Bommarito <michael.bommarito@gmail.com>]

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