Skip to content

Commit

Permalink
[ALSA] Fix races in PCM OSS emulation
Browse files Browse the repository at this point in the history
Fixed the race among multiple threads accessing the OSS PCM
instance concurrently by simply introducing a mutex for protecting
a setup of the PCM.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jaroslav Kysela <perex@suse.cz>
  • Loading branch information
Takashi Iwai authored and Jaroslav Kysela committed Dec 20, 2006
1 parent ba8bdf8 commit e3a5d59
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
1 change: 1 addition & 0 deletions include/sound/pcm_oss.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ struct snd_pcm_oss_runtime {
size_t mmap_bytes;
char *buffer; /* vmallocated period */
size_t buffer_used; /* used length from period buffer */
struct mutex params_lock;
#ifdef CONFIG_SND_PCM_OSS_PLUGINS
struct snd_pcm_plugin *plugin_first;
struct snd_pcm_plugin *plugin_last;
Expand Down
52 changes: 40 additions & 12 deletions sound/core/oss/pcm_oss.c
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,8 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream)
struct snd_mask sformat_mask;
struct snd_mask mask;

if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -EINTR;
sw_params = kmalloc(sizeof(*sw_params), GFP_KERNEL);
params = kmalloc(sizeof(*params), GFP_KERNEL);
sparams = kmalloc(sizeof(*sparams), GFP_KERNEL);
Expand Down Expand Up @@ -1020,6 +1022,7 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream)
kfree(sw_params);
kfree(params);
kfree(sparams);
mutex_unlock(&runtime->oss.params_lock);
return err;
}

Expand Down Expand Up @@ -1307,14 +1310,17 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha

if ((tmp = snd_pcm_oss_make_ready(substream)) < 0)
return tmp;
mutex_lock(&runtime->oss.params_lock);
while (bytes > 0) {
if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) {
tmp = bytes;
if (tmp + runtime->oss.buffer_used > runtime->oss.period_bytes)
tmp = runtime->oss.period_bytes - runtime->oss.buffer_used;
if (tmp > 0) {
if (copy_from_user(runtime->oss.buffer + runtime->oss.buffer_used, buf, tmp))
return xfer > 0 ? (snd_pcm_sframes_t)xfer : -EFAULT;
if (copy_from_user(runtime->oss.buffer + runtime->oss.buffer_used, buf, tmp)) {
tmp = -EFAULT;
goto err;
}
}
runtime->oss.buffer_used += tmp;
buf += tmp;
Expand All @@ -1325,22 +1331,24 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha
tmp = snd_pcm_oss_write2(substream, runtime->oss.buffer + runtime->oss.period_ptr,
runtime->oss.buffer_used - runtime->oss.period_ptr, 1);
if (tmp <= 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
goto err;
runtime->oss.bytes += tmp;
runtime->oss.period_ptr += tmp;
runtime->oss.period_ptr %= runtime->oss.period_bytes;
if (runtime->oss.period_ptr == 0 ||
runtime->oss.period_ptr == runtime->oss.buffer_used)
runtime->oss.buffer_used = 0;
else if ((substream->f_flags & O_NONBLOCK) != 0)
return xfer > 0 ? xfer : -EAGAIN;
else if ((substream->f_flags & O_NONBLOCK) != 0) {
tmp = -EAGAIN;
goto err;
}
}
} else {
tmp = snd_pcm_oss_write2(substream,
(const char __force *)buf,
runtime->oss.period_bytes, 0);
if (tmp <= 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
goto err;
runtime->oss.bytes += tmp;
buf += tmp;
bytes -= tmp;
Expand All @@ -1350,7 +1358,12 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha
break;
}
}
mutex_unlock(&runtime->oss.params_lock);
return xfer;

err:
mutex_unlock(&runtime->oss.params_lock);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
}

static ssize_t snd_pcm_oss_read2(struct snd_pcm_substream *substream, char *buf, size_t bytes, int in_kernel)
Expand Down Expand Up @@ -1397,21 +1410,24 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use

if ((tmp = snd_pcm_oss_make_ready(substream)) < 0)
return tmp;
mutex_lock(&runtime->oss.params_lock);
while (bytes > 0) {
if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) {
if (runtime->oss.buffer_used == 0) {
tmp = snd_pcm_oss_read2(substream, runtime->oss.buffer, runtime->oss.period_bytes, 1);
if (tmp <= 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
goto err;
runtime->oss.bytes += tmp;
runtime->oss.period_ptr = tmp;
runtime->oss.buffer_used = tmp;
}
tmp = bytes;
if ((size_t) tmp > runtime->oss.buffer_used)
tmp = runtime->oss.buffer_used;
if (copy_to_user(buf, runtime->oss.buffer + (runtime->oss.period_ptr - runtime->oss.buffer_used), tmp))
return xfer > 0 ? (snd_pcm_sframes_t)xfer : -EFAULT;
if (copy_to_user(buf, runtime->oss.buffer + (runtime->oss.period_ptr - runtime->oss.buffer_used), tmp)) {
tmp = -EFAULT;
goto err;
}
buf += tmp;
bytes -= tmp;
xfer += tmp;
Expand All @@ -1420,14 +1436,19 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use
tmp = snd_pcm_oss_read2(substream, (char __force *)buf,
runtime->oss.period_bytes, 0);
if (tmp <= 0)
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
goto err;
runtime->oss.bytes += tmp;
buf += tmp;
bytes -= tmp;
xfer += tmp;
}
}
mutex_unlock(&runtime->oss.params_lock);
return xfer;

err:
mutex_unlock(&runtime->oss.params_lock);
return xfer > 0 ? (snd_pcm_sframes_t)xfer : tmp;
}

static int snd_pcm_oss_reset(struct snd_pcm_oss_file *pcm_oss_file)
Expand Down Expand Up @@ -1528,6 +1549,7 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
return err;
format = snd_pcm_oss_format_from(runtime->oss.format);
width = snd_pcm_format_physical_width(format);
mutex_lock(&runtime->oss.params_lock);
if (runtime->oss.buffer_used > 0) {
#ifdef OSS_DEBUG
printk("sync: buffer_used\n");
Expand All @@ -1537,8 +1559,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
runtime->oss.buffer + runtime->oss.buffer_used,
size);
err = snd_pcm_oss_sync1(substream, runtime->oss.period_bytes);
if (err < 0)
if (err < 0) {
mutex_unlock(&runtime->oss.params_lock);
return err;
}
} else if (runtime->oss.period_ptr > 0) {
#ifdef OSS_DEBUG
printk("sync: period_ptr\n");
Expand All @@ -1548,8 +1572,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
runtime->oss.buffer,
size * 8 / width);
err = snd_pcm_oss_sync1(substream, size);
if (err < 0)
if (err < 0) {
mutex_unlock(&runtime->oss.params_lock);
return err;
}
}
/*
* The ALSA's period might be a bit large than OSS one.
Expand Down Expand Up @@ -1579,6 +1605,7 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
snd_pcm_lib_writev(substream, buffers, size);
}
}
mutex_unlock(&runtime->oss.params_lock);
/*
* finish sync: drain the buffer
*/
Expand Down Expand Up @@ -2172,6 +2199,7 @@ static void snd_pcm_oss_init_substream(struct snd_pcm_substream *substream,
runtime->oss.params = 1;
runtime->oss.trigger = 1;
runtime->oss.rate = 8000;
mutex_init(&runtime->oss.params_lock);
switch (SNDRV_MINOR_OSS_DEVICE(minor)) {
case SNDRV_MINOR_OSS_PCM_8:
runtime->oss.format = AFMT_U8;
Expand Down

0 comments on commit e3a5d59

Please sign in to comment.