Skip to content

Commit 826af7f

Browse files
committed
ALSA: aloop: Fix racy access at PCM trigger
The PCM trigger callback of aloop driver tries to check the PCM state and stop the stream of the tied substream in the corresponding cable. Since both check and stop operations are performed outside the cable lock, this may result in UAF when a program attempts to trigger frequently while opening/closing the tied stream, as spotted by fuzzers. For addressing the UAF, this patch changes two things: - It covers the most of code in loopback_check_format() with cable->lock spinlock, and add the proper NULL checks. This avoids already some racy accesses. - In addition, now we try to check the state of the capture PCM stream that may be stopped in this function, which was the major pain point leading to UAF. Reported-by: syzbot+5f8f3acdee1ec7a7ef7b@syzkaller.appspotmail.com Closes: https://lore.kernel.org/69783ba1.050a0220.c9109.0011.GAE@google.com Cc: <stable@vger.kernel.org> Link: https://patch.msgid.link/20260203141003.116584-1-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
1 parent 00f32df commit 826af7f

1 file changed

Lines changed: 36 additions & 26 deletions

File tree

sound/drivers/aloop.c

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -336,37 +336,43 @@ static bool is_access_interleaved(snd_pcm_access_t access)
336336

337337
static int loopback_check_format(struct loopback_cable *cable, int stream)
338338
{
339+
struct loopback_pcm *dpcm_play, *dpcm_capt;
339340
struct snd_pcm_runtime *runtime, *cruntime;
340341
struct loopback_setup *setup;
341342
struct snd_card *card;
343+
bool stop_capture = false;
342344
int check;
343345

344-
if (cable->valid != CABLE_VALID_BOTH) {
345-
if (stream == SNDRV_PCM_STREAM_PLAYBACK)
346-
goto __notify;
347-
return 0;
348-
}
349-
runtime = cable->streams[SNDRV_PCM_STREAM_PLAYBACK]->
350-
substream->runtime;
351-
cruntime = cable->streams[SNDRV_PCM_STREAM_CAPTURE]->
352-
substream->runtime;
353-
check = runtime->format != cruntime->format ||
354-
runtime->rate != cruntime->rate ||
355-
runtime->channels != cruntime->channels ||
356-
is_access_interleaved(runtime->access) !=
357-
is_access_interleaved(cruntime->access);
358-
if (!check)
359-
return 0;
360-
if (stream == SNDRV_PCM_STREAM_CAPTURE) {
361-
return -EIO;
362-
} else {
363-
snd_pcm_stop(cable->streams[SNDRV_PCM_STREAM_CAPTURE]->
364-
substream, SNDRV_PCM_STATE_DRAINING);
365-
__notify:
366-
runtime = cable->streams[SNDRV_PCM_STREAM_PLAYBACK]->
367-
substream->runtime;
368-
setup = get_setup(cable->streams[SNDRV_PCM_STREAM_PLAYBACK]);
369-
card = cable->streams[SNDRV_PCM_STREAM_PLAYBACK]->loopback->card;
346+
scoped_guard(spinlock_irqsave, &cable->lock) {
347+
dpcm_play = cable->streams[SNDRV_PCM_STREAM_PLAYBACK];
348+
dpcm_capt = cable->streams[SNDRV_PCM_STREAM_CAPTURE];
349+
350+
if (cable->valid != CABLE_VALID_BOTH) {
351+
if (stream == SNDRV_PCM_STREAM_CAPTURE || !dpcm_play)
352+
return 0;
353+
} else {
354+
if (!dpcm_play || !dpcm_capt)
355+
return -EIO;
356+
runtime = dpcm_play->substream->runtime;
357+
cruntime = dpcm_capt->substream->runtime;
358+
if (!runtime || !cruntime)
359+
return -EIO;
360+
check = runtime->format != cruntime->format ||
361+
runtime->rate != cruntime->rate ||
362+
runtime->channels != cruntime->channels ||
363+
is_access_interleaved(runtime->access) !=
364+
is_access_interleaved(cruntime->access);
365+
if (!check)
366+
return 0;
367+
if (stream == SNDRV_PCM_STREAM_CAPTURE)
368+
return -EIO;
369+
else if (cruntime->state == SNDRV_PCM_STATE_RUNNING)
370+
stop_capture = true;
371+
}
372+
373+
setup = get_setup(dpcm_play);
374+
card = dpcm_play->loopback->card;
375+
runtime = dpcm_play->substream->runtime;
370376
if (setup->format != runtime->format) {
371377
snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE,
372378
&setup->format_id);
@@ -389,6 +395,10 @@ static int loopback_check_format(struct loopback_cable *cable, int stream)
389395
setup->access = runtime->access;
390396
}
391397
}
398+
399+
if (stop_capture)
400+
snd_pcm_stop(dpcm_capt->substream, SNDRV_PCM_STATE_DRAINING);
401+
392402
return 0;
393403
}
394404

0 commit comments

Comments
 (0)