include/sound/pcm.h | 4 ++++ sound/core/oss/pcm_oss.c | 44 +++++++++++++++++++++++----------------- sound/core/pcm_native.c | 23 +++++++++++++++++++-- 3 files changed, 50 insertions(+), 21 deletions(-)
__snd_pcm_set_state() writes runtime->state under the PCM stream lock.
However, the OSS I/O functions snd_pcm_oss_write3(), snd_pcm_oss_read3(),
snd_pcm_oss_writev3() and snd_pcm_oss_readv3() read runtime->state
without holding the stream lock, only holding oss.params_lock (a
different mutex that does not synchronize with the stream lock).
Since __snd_pcm_set_state() is called from IRQ context (e.g.,
snd_pcm_period_elapsed -> snd_pcm_update_state -> __snd_pcm_xrun ->
snd_pcm_stop -> snd_pcm_post_stop) while the OSS read/write paths
run in process context, these are concurrent accesses that constitute
a data race.
Rather than using READ_ONCE()/WRITE_ONCE() barriers, introduce a
snd_pcm_get_state() helper that reads runtime->state under the stream
lock, matching the locking discipline used elsewhere in the PCM layer.
Also export snd_pcm_set_state() for completeness.
Use snd_pcm_get_state() in all four OSS I/O functions, caching the
result in a local variable where the same snapshot is used for
multiple comparisons to avoid taking the lock repeatedly.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
include/sound/pcm.h | 4 ++++
sound/core/oss/pcm_oss.c | 44 +++++++++++++++++++++++-----------------
sound/core/pcm_native.c | 23 +++++++++++++++++++--
3 files changed, 50 insertions(+), 21 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index a7860c047503..76fc33dce537 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -729,6 +729,10 @@ static inline void __snd_pcm_set_state(struct snd_pcm_runtime *runtime,
runtime->status->state = state; /* copy for mmap */
}
+void snd_pcm_set_state(struct snd_pcm_substream *substream,
+ snd_pcm_state_t state);
+snd_pcm_state_t snd_pcm_get_state(struct snd_pcm_substream *substream);
+
/**
* bytes_to_samples - Unit conversion of the size from bytes to samples
* @runtime: PCM runtime instance
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index d4fd4dfc7fc3..a140a0d9abb8 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1227,14 +1227,16 @@ static int snd_pcm_oss_capture_position_fixup(struct snd_pcm_substream *substrea
snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const char *ptr, snd_pcm_uframes_t frames, int in_kernel)
{
struct snd_pcm_runtime *runtime = substream->runtime;
+ snd_pcm_state_t state;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ state = snd_pcm_get_state(substream);
+ if (state == SNDRV_PCM_STATE_XRUN ||
+ state == SNDRV_PCM_STATE_SUSPENDED) {
#ifdef OSS_DEBUG
pcm_dbg(substream->pcm,
"pcm_oss: write: recovering from %s\n",
- runtime->state == SNDRV_PCM_STATE_XRUN ?
+ state == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_oss_prepare(substream);
@@ -1249,7 +1251,7 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const
break;
/* test, if we can't store new data, because the stream */
/* has not been started */
- if (runtime->state == SNDRV_PCM_STATE_PREPARED)
+ if (snd_pcm_get_state(substream) == SNDRV_PCM_STATE_PREPARED)
return -EAGAIN;
}
return ret;
@@ -1259,20 +1261,22 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
{
struct snd_pcm_runtime *runtime = substream->runtime;
snd_pcm_sframes_t delay;
+ snd_pcm_state_t state;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ state = snd_pcm_get_state(substream);
+ if (state == SNDRV_PCM_STATE_XRUN ||
+ state == SNDRV_PCM_STATE_SUSPENDED) {
#ifdef OSS_DEBUG
pcm_dbg(substream->pcm,
"pcm_oss: read: recovering from %s\n",
- runtime->state == SNDRV_PCM_STATE_XRUN ?
+ state == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DRAIN, NULL);
if (ret < 0)
break;
- } else if (runtime->state == SNDRV_PCM_STATE_SETUP) {
+ } else if (state == SNDRV_PCM_STATE_SETUP) {
ret = snd_pcm_oss_prepare(substream);
if (ret < 0)
break;
@@ -1285,7 +1289,7 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
frames, in_kernel);
mutex_lock(&runtime->oss.params_lock);
if (ret == -EPIPE) {
- if (runtime->state == SNDRV_PCM_STATE_DRAINING) {
+ if (snd_pcm_get_state(substream) == SNDRV_PCM_STATE_DRAINING) {
ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
if (ret < 0)
break;
@@ -1301,15 +1305,16 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
#ifdef CONFIG_SND_PCM_OSS_PLUGINS
snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames)
{
- struct snd_pcm_runtime *runtime = substream->runtime;
+ snd_pcm_state_t state;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ state = snd_pcm_get_state(substream);
+ if (state == SNDRV_PCM_STATE_XRUN ||
+ state == SNDRV_PCM_STATE_SUSPENDED) {
#ifdef OSS_DEBUG
pcm_dbg(substream->pcm,
"pcm_oss: writev: recovering from %s\n",
- runtime->state == SNDRV_PCM_STATE_XRUN ?
+ state == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_oss_prepare(substream);
@@ -1322,7 +1327,7 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void
/* test, if we can't store new data, because the stream */
/* has not been started */
- if (runtime->state == SNDRV_PCM_STATE_PREPARED)
+ if (snd_pcm_get_state(substream) == SNDRV_PCM_STATE_PREPARED)
return -EAGAIN;
}
return ret;
@@ -1330,21 +1335,22 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void
snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames)
{
- struct snd_pcm_runtime *runtime = substream->runtime;
+ snd_pcm_state_t state;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ state = snd_pcm_get_state(substream);
+ if (state == SNDRV_PCM_STATE_XRUN ||
+ state == SNDRV_PCM_STATE_SUSPENDED) {
#ifdef OSS_DEBUG
pcm_dbg(substream->pcm,
"pcm_oss: readv: recovering from %s\n",
- runtime->state == SNDRV_PCM_STATE_XRUN ?
+ state == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DRAIN, NULL);
if (ret < 0)
break;
- } else if (runtime->state == SNDRV_PCM_STATE_SETUP) {
+ } else if (state == SNDRV_PCM_STATE_SETUP) {
ret = snd_pcm_oss_prepare(substream);
if (ret < 0)
break;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 5a64453da728..674b50c7c5f6 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -618,13 +618,32 @@ static int period_to_usecs(struct snd_pcm_runtime *runtime)
return usecs;
}
-static void snd_pcm_set_state(struct snd_pcm_substream *substream,
- snd_pcm_state_t state)
+/**
+ * snd_pcm_set_state - Set the PCM runtime state with stream lock
+ * @substream: PCM substream
+ * @state: state to set
+ */
+void snd_pcm_set_state(struct snd_pcm_substream *substream,
+ snd_pcm_state_t state)
{
guard(pcm_stream_lock_irq)(substream);
if (substream->runtime->state != SNDRV_PCM_STATE_DISCONNECTED)
__snd_pcm_set_state(substream->runtime, state);
}
+EXPORT_SYMBOL_GPL(snd_pcm_set_state);
+
+/**
+ * snd_pcm_get_state - Read the PCM runtime state with stream lock
+ * @substream: PCM substream
+ *
+ * Return: the current PCM state
+ */
+snd_pcm_state_t snd_pcm_get_state(struct snd_pcm_substream *substream)
+{
+ guard(pcm_stream_lock_irqsave)(substream);
+ return substream->runtime->state;
+}
+EXPORT_SYMBOL_GPL(snd_pcm_get_state);
static inline void snd_pcm_timer_notify(struct snd_pcm_substream *substream,
int event)
--
2.34.1On Mon, 16 Mar 2026 09:50:47 +0100, Cen Zhang wrote: > > __snd_pcm_set_state() writes runtime->state under the PCM stream lock. > However, the OSS I/O functions snd_pcm_oss_write3(), snd_pcm_oss_read3(), > snd_pcm_oss_writev3() and snd_pcm_oss_readv3() read runtime->state > without holding the stream lock, only holding oss.params_lock (a > different mutex that does not synchronize with the stream lock). > > Since __snd_pcm_set_state() is called from IRQ context (e.g., > snd_pcm_period_elapsed -> snd_pcm_update_state -> __snd_pcm_xrun -> > snd_pcm_stop -> snd_pcm_post_stop) while the OSS read/write paths > run in process context, these are concurrent accesses that constitute > a data race. > > Rather than using READ_ONCE()/WRITE_ONCE() barriers, introduce a > snd_pcm_get_state() helper that reads runtime->state under the stream > lock, matching the locking discipline used elsewhere in the PCM layer. > Also export snd_pcm_set_state() for completeness. > > Use snd_pcm_get_state() in all four OSS I/O functions, caching the > result in a local variable where the same snapshot is used for > multiple comparisons to avoid taking the lock repeatedly. > > Signed-off-by: Cen Zhang <zzzccc427@gmail.com> Applied to for-next branch now. Thanks. Takashi
© 2016 - 2026 Red Hat, Inc.