Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] ALSA: virtio: sub-byte frame truncation in virtsnd_pcm_capture_ack_validate() --
commit 6d8079ee9be8b0bc45d98d37834441cf9aa4b749 Author: Joonho Lee <[email protected]> ALSA: virtio: validate indirect capture appl_ptr accounting This commit retains references to PCM DMA pages (or uses a driver-owned buffer on NOMMU), validates full-period capture responses, seeds the RX ring once per prepare, and validates capture appl_ptr accounting before invoking the indirect transfer helper. > diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c > index 343700ec50476..6a8752997d092 100644 > --- a/sound/virtio/virtio_pcm_ops.c > +++ b/sound/virtio/virtio_pcm_ops.c [ ... ] > @@ -599,6 +619,69 @@ struct virtsnd_pcm_transfer { > int error; > }; > > +/* Caller holds @vss->lock and the PCM stream lock. */ > +static int virtsnd_pcm_capture_ack_validate(struct virtio_pcm_substream *vss, > + struct snd_pcm_runtime *runtime, > + bool *skip_transfer) > +{ [ ... ] > + /* The RX ring reserves its full size; only the excess is unread data. > */ > + unread_bytes = (s64)rec->hw_ready + (s64)rec->sw_ready - > + (s64)rec->sw_buffer_size; > + if (unread_bytes < 0 || unread_bytes > (s64)hw_queue_size) > + return -EPIPE; > + > + appl_bytes = frames_to_bytes(runtime, (snd_pcm_uframes_t)appl_diff); > + if ((u64)appl_bytes > (u64)unread_bytes) > + return -EPIPE; > + > + return 0; > +} [Severity: Medium] Can frames_to_bytes() truncate sub-byte frame deltas here and let an unaligned appl_ptr advance bypass validation? virtsnd_pcm_hw_params() permits sub-byte frame formats such as 1-channel SNDRV_PCM_FORMAT_IMA_ADPCM (4-bit frames, runtime->min_align == 2) when the period size is a whole number of bytes. If userspace advances appl_ptr by an odd number of frames via SNDRV_PCM_IOCTL_SYNC_PTR (for example, 1 frame when unread_bytes is 0), frames_to_bytes() truncates appl_bytes down, passing the unread_bytes check. snd_pcm_indirect_capture_transfer() then sets rec->appl_ptr to the unaligned appl_ptr while subtracting the truncated byte count from rec->sw_ready, pushing appl_ptr past hw_ptr and permanently leaking rec->sw_ready accounting so RX descriptors stop being requeued. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
