mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-05-16 02:01:18 -04:00
Code Issues Packages Projects Releases Wiki Activity

ALSA: pcm: oss: use proper stream lock for runtime->state access

Browse Source 
__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>
Link: https://patch.msgid.link/20260316085047.2876451-1-zzzccc427@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
Cen Zhang
2026-03-16 16:50:47 +08:00
committed by Takashi Iwai
parent 103a7b97c6
commit 032322b44c
3 changed files with 50 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
include/sound/pcm.h
View File

@@ -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

44
sound/core/oss/pcm_oss.c
View File

@@ -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;

23
sound/core/pcm_native.c
View File

@@ -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)