From ebe4f5f49f58221fbc9a39efcd3206cbbc071b9e Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 29 Mar 2022 15:54:59 +0200 Subject: [PATCH] Revert "Revert "ALSA: pcm: Fix races among concurrent hw_params and hw_free calls"" This reverts commit 162cbdd8076b702e81cc1bcb20eece70114f4d2f. It fixes CVE-2022-1048 and it had to originally be reverted due to ABI issues. Add it back now as the ABI "break" is allowed and we want to fix this real problem. Signed-off-by: Greg Kroah-Hartman Change-Id: I230096f0f827d4a49b0574a5cf27667ef132d5d9 --- include/sound/pcm.h | 1 + sound/core/pcm.c | 2 ++ sound/core/pcm_native.c | 61 ++++++++++++++++++++++++++--------------- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 33451f8ff755..2018b7512d3d 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -398,6 +398,7 @@ struct snd_pcm_runtime { wait_queue_head_t tsleep; /* transfer sleep */ struct fasync_struct *fasync; bool stop_operating; /* sync_stop will be called */ + struct mutex buffer_mutex; /* protect for buffer changes */ /* -- private section -- */ void *private_data; diff --git a/sound/core/pcm.c b/sound/core/pcm.c index ba4a987ed1c6..edd9849210f2 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -969,6 +969,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, init_waitqueue_head(&runtime->tsleep); runtime->status->state = SNDRV_PCM_STATE_OPEN; + mutex_init(&runtime->buffer_mutex); substream->runtime = runtime; substream->private_data = pcm->private_data; @@ -1002,6 +1003,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) } else { substream->runtime = NULL; } + mutex_destroy(&runtime->buffer_mutex); kfree(runtime); put_pid(substream->pid); substream->pid = NULL; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index b655ecb672d5..9ea2dbbd91aa 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -672,33 +672,40 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, return 0; } +#if IS_ENABLED(CONFIG_SND_PCM_OSS) +#define is_oss_stream(substream) ((substream)->oss.oss) +#else +#define is_oss_stream(substream) false +#endif + static int snd_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_pcm_runtime *runtime; - int err, usecs; + int err = 0, usecs; unsigned int bits; snd_pcm_uframes_t frames; if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; + mutex_lock(&runtime->buffer_mutex); snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_OPEN: case SNDRV_PCM_STATE_SETUP: case SNDRV_PCM_STATE_PREPARED: + if (!is_oss_stream(substream) && + atomic_read(&substream->mmap_count)) + err = -EBADFD; break; default: - snd_pcm_stream_unlock_irq(substream); - return -EBADFD; + err = -EBADFD; + break; } snd_pcm_stream_unlock_irq(substream); -#if IS_ENABLED(CONFIG_SND_PCM_OSS) - if (!substream->oss.oss) -#endif - if (atomic_read(&substream->mmap_count)) - return -EBADFD; + if (err) + goto unlock; snd_pcm_sync_stop(substream, true); @@ -786,16 +793,21 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, if (usecs >= 0) cpu_latency_qos_add_request(&substream->latency_pm_qos_req, usecs); - return 0; + err = 0; _error: - /* hardware might be unusable from this time, - so we force application to retry to set - the correct hardware parameter settings */ - snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); - if (substream->ops->hw_free != NULL) - substream->ops->hw_free(substream); - if (substream->managed_buffer_alloc) - snd_pcm_lib_free_pages(substream); + if (err) { + /* hardware might be unusable from this time, + * so we force application to retry to set + * the correct hardware parameter settings + */ + snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); + if (substream->ops->hw_free != NULL) + substream->ops->hw_free(substream); + if (substream->managed_buffer_alloc) + snd_pcm_lib_free_pages(substream); + } + unlock: + mutex_unlock(&runtime->buffer_mutex); return err; } @@ -835,26 +847,31 @@ static int do_hw_free(struct snd_pcm_substream *substream) static int snd_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime; - int result; + int result = 0; if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; + mutex_lock(&runtime->buffer_mutex); snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_SETUP: case SNDRV_PCM_STATE_PREPARED: + if (atomic_read(&substream->mmap_count)) + result = -EBADFD; break; default: - snd_pcm_stream_unlock_irq(substream); - return -EBADFD; + result = -EBADFD; + break; } snd_pcm_stream_unlock_irq(substream); - if (atomic_read(&substream->mmap_count)) - return -EBADFD; + if (result) + goto unlock; result = do_hw_free(substream); snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); cpu_latency_qos_remove_request(&substream->latency_pm_qos_req); + unlock: + mutex_unlock(&runtime->buffer_mutex); return result; }