From: Aohan Mei snd_pcm_hw_params() and snd_pcm_hw_free() validate the stream state only once, under the stream lock at entry, and then free/realloc the DMA buffer (snd_pcm_lib_free_pages() -> snd_pcm_set_runtime_buffer(NULL), followed by the sleeping allocation in snd_pcm_lib_malloc_pages()) and update the runtime fields without holding the stream lock. The state, however, stays SNDRV_PCM_STATE_PREPARED until the final snd_pcm_set_state() at the very end of the operation. A concurrent SNDRV_PCM_IOCTL_START takes only the stream lock and merely requires state == PREPARED in snd_pcm_pre_start(), so it can slip into that window: snd_pcm_post_start() sets the state RUNNING, arms the driver data plane and fills the initial silence through snd_pcm_playback_silence(). The data plane then keeps operating on the buffer that hw_params/hw_free is tearing down concurrently; once snd_pcm_set_runtime_buffer(NULL) has cleared runtime->dma_area, the silence fill in fill_silence() -> get_dma_ptr() dereferences a NULL pointer, and on real hardware the device may additionally keep DMA-ing into the freed pages. Close the race by leaving the PREPARED state atomically with the entry state check, inside the same stream lock critical section: a START that already completed makes the hw_params/hw_free state check fail with -EBADFD, and a later START observes SETUP and fails in snd_pcm_pre_start() with -EBADFD as well, so the data plane can never be armed while the buffer is being freed or reallocated. Both functions impose SETUP as their resulting state anyway, hence this does not change the state machine semantics visible to user space. The buffer_mutex/buffer_accessing serialization introduced by the earlier fixes for the prepare-vs-hw_params races cannot simply be extended to the START path: the trigger action has to run under the IRQ-off stream spinlock for atomic PCMs, and failing START with -EBUSY whenever a read/write transfer is in flight would be a user visible regression. Transitioning the state atomically at entry avoids both problems. Cc: stable@vger.kernel.org Assisted-by: CodeBuddy:Kimi-K3 Signed-off-by: Aohan Mei --- sound/core/pcm_native.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 62324282fcae..6040eb752873 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -805,6 +805,17 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, err = -EBADFD; break; } + /* + * The buffer free/realloc and the runtime field updates + * below run without the stream lock, while a concurrent + * SNDRV_PCM_IOCTL_START only requires state == PREPARED + * under that lock. Leave the PREPARED state atomically + * with the check above so that a racing START (and the + * data plane it arms) can no longer slip in and operate + * on the buffer while it is being freed or reallocated. + */ + if (!err && runtime->state == SNDRV_PCM_STATE_PREPARED) + __snd_pcm_set_state(runtime, SNDRV_PCM_STATE_SETUP); } if (err) goto unlock; @@ -966,6 +977,13 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream) result = -EBADFD; break; } + /* Same race as in snd_pcm_hw_params(): leave the PREPARED + * state atomically with the check above, so that a racing + * START cannot arm the data plane while do_hw_free() tears + * down the buffer without the stream lock. + */ + if (!result && runtime->state == SNDRV_PCM_STATE_PREPARED) + __snd_pcm_set_state(runtime, SNDRV_PCM_STATE_SETUP); } if (result) goto unlock; -- 2.43.7