From: Cen Zhang <zzzccc427@gmail.com>
To: perex@perex.cz, chleroy@kernel.org
Cc: tiwai@suse.com, linux-sound@vger.kernel.org,
linux-kernel@vger.kernel.org, baijiaju1990@gmail.com,
r33s3n6@gmail.com, gality369@gmail.com, zhenghaoran154@gmail.com,
hanguidong02@gmail.com, ziyuzhang201@gmail.com,
Cen Zhang <zzzccc427@gmail.com>
Subject: [PATCH v2] ALSA: pcm: oss: use proper stream lock for runtime->state access
Date: Mon, 16 Mar 2026 16:50:47 +0800 [thread overview]
Message-ID: <20260316085047.2876451-1-zzzccc427@gmail.com> (raw)
__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.1
next reply other threads:[~2026-03-16 9:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 8:50 Cen Zhang [this message]
2026-03-16 17:06 ` [PATCH v2] ALSA: pcm: oss: use proper stream lock for runtime->state access Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260316085047.2876451-1-zzzccc427@gmail.com \
--to=zzzccc427@gmail.com \
--cc=baijiaju1990@gmail.com \
--cc=chleroy@kernel.org \
--cc=gality369@gmail.com \
--cc=hanguidong02@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=perex@perex.cz \
--cc=r33s3n6@gmail.com \
--cc=tiwai@suse.com \
--cc=zhenghaoran154@gmail.com \
--cc=ziyuzhang201@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox