linux-kernel - [RFC] ALSA: usb-audio: Fix missing xrun report in lowlatency mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <25d5b0d8-4efd-4630-9d33-7a9e3fa9dc2b@gmail.com>
Date: Tue, 19 Nov 2024 23:54:19 +0200
From: Leonard Crestez <cdleonard@...il.com>
To: Takashi Iwai <tiwai@...e.com>, linux-sound@...r.kernel.org
Cc: Jaroslav Kysela <perex@...ex.cz>, linux-kernel@...r.kernel.org
Subject: [RFC] ALSA: usb-audio: Fix missing xrun report in lowlatency mode

Hello,

I’m investigating an issue where USB Audio does not properly send XRUN 
notifications.

The issue can be reproduced with aplay: enable xrun_debug, aplay -D 
plughw:0 and CTRL-Z - no XRUN message is seen

Disabling lowlatency_playback via modprobe parameter does make this 
issue go away - XRUNs are reported correctly without any changes.

After a lot of tracing the following seems to be happening:

- prepare_playback_urb find avail=48, meaning 48 bytes still to-be-played
- snd_usb_endpoint_next_packet_size decides that 48 is too little and 
returns -EAGAIN. Specifically -EAGAIN is returned from next_packet_size
- The return value of prepare_playback_urb is propagated through 
prepare_outbound_urb back to snd_usb_queue_pending_output_urbs
- snd_usb_queue_pending_output_urbs receives -EAGAIN from 
prepare_outbound_urb
- since err is -EAGAIN the ctx is pushed back to the ready list and 
transmission is aborted but notify_xrun is skipped
- no more playback?

It is possible to make XRUNs happen by caling notify_xrun even on 
-EAGAIN, diff looks like this:

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 568099467dbb..da64ee0cf60a 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -495,10 +495,11 @@ int snd_usb_queue_pending_output_urbs(struct 
snd_usb_endpoint *ep,
                         break;
                 if (err < 0) {
                         /* push back to ready list again for -EAGAIN */
                         if (err == -EAGAIN) {
                                 push_back_to_ready_list(ep, ctx);
+                               notify_xrun(ep);
                                 break;
                         }

                         if (!in_stream_lock)
                                 notify_xrun(ep);

This mail was not formatted as proper patch because this seems very 
likely incorrect, it undoes an explicit check. What would a correct 
solution look like?

There could be a scenario where -EAGAIN from prepare_outbound_urb can 
happen without an actual XRUN, but I don’t understand the code enough.

The fact that usb-audio halts playback with a non-zero amount of bytes 
that can still be played prevents the XRUN checks inside 
`snd_pcm_update_state` from triggering. This means that the driver 
always takes responsibility for reporting xrun, correct?

It seems dubious to me that unplayed bytes are kept around - shouldn’t 
fragments either be played or dropped? Perhaps the last fragment should 
just be padded with some silence and sent anyway, then core sound code 
should report xrun anyway.

USB Audio has many parameters. In my particular case:

- lowlatency_playback = true
- channels=2, rate=48000, format=S16_LE, period_bytes=960, periods=3, 
implicit_fb=0
- nurbs = 3

This was originally found in a complex application which checks 
snd_pcm_state regularly and does not find the SND_PCM_STATE_XRUN state 
but audio gaps are seen in a physical line capture.

This was seen in Linux 5.19.139 but also affects 6.6.y and latest 
sound/master branch. There are very few relevant changes in this area 
that are not already in stable branches.

Hardware is a conexant audio codec, USB VID:PID 0572:1410.

--
Regards,
Leonard