From: Cen Zhang <zzzccc427@gmail.com>
To: perex@perex.cz, tiwai@suse.com, chleroy@kernel.org
Cc: 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] ALSA: pcm: oss: annotate data-races around runtime->state
Date: Mon, 16 Mar 2026 11:05:50 +0800 [thread overview]
Message-ID: <20260316030550.2848349-1-zzzccc427@gmail.com> (raw)
__snd_pcm_set_state() writes runtime->state under the PCM stream lock:
runtime->state = state;
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):
if (runtime->state == SNDRV_PCM_STATE_XRUN || ...)
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.
The code handles stale reads gracefully through its retry loop
(re-checking after __snd_pcm_lib_xfer returns -EPIPE), so the race
is not harmful under simple interleaving. However, plain C accesses
are formally undefined under LKMM, and without READ_ONCE the compiler
is free to fuse or cache the loads across loop iterations.
Add WRITE_ONCE() in __snd_pcm_set_state() for the write side and
READ_ONCE() on all lockless reads of runtime->state in the four OSS
I/O functions.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
include/sound/pcm.h | 2 +-
sound/core/oss/pcm_oss.c | 34 +++++++++++++++++-----------------
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index a7860c047503..a91061ace828 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -725,7 +725,7 @@ static inline int snd_pcm_running(struct snd_pcm_substream *substream)
static inline void __snd_pcm_set_state(struct snd_pcm_runtime *runtime,
snd_pcm_state_t state)
{
- runtime->state = state;
+ WRITE_ONCE(runtime->state, state);
runtime->status->state = state; /* copy for mmap */
}
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index d4fd4dfc7fc3..b9277f54fa27 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1229,12 +1229,12 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const
struct snd_pcm_runtime *runtime = substream->runtime;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ if (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ||
+ READ_ONCE(runtime->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 ?
+ READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_oss_prepare(substream);
@@ -1249,7 +1249,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 (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_PREPARED)
return -EAGAIN;
}
return ret;
@@ -1261,18 +1261,18 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p
snd_pcm_sframes_t delay;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ if (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ||
+ READ_ONCE(runtime->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 ?
+ READ_ONCE(runtime->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 (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_SETUP) {
ret = snd_pcm_oss_prepare(substream);
if (ret < 0)
break;
@@ -1285,7 +1285,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 (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_DRAINING) {
ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
if (ret < 0)
break;
@@ -1304,12 +1304,12 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void
struct snd_pcm_runtime *runtime = substream->runtime;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ if (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ||
+ READ_ONCE(runtime->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 ?
+ READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ?
"XRUN" : "SUSPEND");
#endif
ret = snd_pcm_oss_prepare(substream);
@@ -1322,7 +1322,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 (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_PREPARED)
return -EAGAIN;
}
return ret;
@@ -1333,18 +1333,18 @@ snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void *
struct snd_pcm_runtime *runtime = substream->runtime;
int ret;
while (1) {
- if (runtime->state == SNDRV_PCM_STATE_XRUN ||
- runtime->state == SNDRV_PCM_STATE_SUSPENDED) {
+ if (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_XRUN ||
+ READ_ONCE(runtime->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 ?
+ READ_ONCE(runtime->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 (READ_ONCE(runtime->state) == SNDRV_PCM_STATE_SETUP) {
ret = snd_pcm_oss_prepare(substream);
if (ret < 0)
break;
--
2.34.1
next reply other threads:[~2026-03-16 3:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 3:05 Cen Zhang [this message]
2026-03-16 8:40 ` [PATCH] ALSA: pcm: oss: annotate data-races around runtime->state Takashi Iwai
2026-03-16 9:07 ` Cen Zhang
-- strict thread matches above, loose matches on Subject: below --
2026-03-16 6:48 Cen Zhang
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=20260316030550.2848349-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