Skip to content

L
linux
Commit 02a5d692 authored by Takashi Iwai's avatar Takashi Iwai
Browse files
Options

ALSA: pcm: Avoid potential races between OSS ioctls and read/write 

Although we apply the params_lock mutex to the whole read and write
operations as well as snd_pcm_oss_change_params(), we may still face
some races.

First off, the params_lock is taken inside the read and write loop.
This is intentional for avoiding the too long locking, but it allows
the in-between parameter change, which might lead to invalid
pointers.  We check the readiness of the stream and set up via
snd_pcm_oss_make_ready() at the beginning of read and write, but it's
called only once, by assuming that it remains ready in the rest.

Second, many ioctls that may change the actual parameters
(i.e. setting runtime->oss.params=1) aren't protected, hence they can
be processed in a half-baked state.

This patch is an attempt to plug these holes.  The stream readiness
check is moved inside the read/write inner loop, so that the stream is
always set up in a proper state before further processing.  Also, each
ioctl that may change the parameter is wrapped with the params_lock
for avoiding the races.

The issues were triggered by syzkaller in a few different scenarios,
particularly the one below appearing as GPF in loopback_pos_update.

Reported-by: syzbot+c4227aec125487ec3efa@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent f3b906d7
Show whitespace changes
Inline Side-by-side
Showing with 106 additions and 28 deletions
sound/core/oss/pcm_oss.c
View file @ 02a5d692
...@@ -823,8 +823,8 @@ static int choose_rate(struct snd_pcm_substream *substream, ...@@ -823,8 +823,8 @@ static int choose_rate(struct snd_pcm_substream *substream,
return snd_pcm_hw_param_near(substream, params, SNDRV_PCM_HW_PARAM_RATE, best_rate, NULL); return snd_pcm_hw_param_near(substream, params, SNDRV_PCM_HW_PARAM_RATE, best_rate, NULL);
} }
static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream, /* call with params_lock held */
bool trylock) static int snd_pcm_oss_change_params_locked(struct snd_pcm_substream *substream)
{ {
struct snd_pcm_runtime *runtime = substream->runtime; struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_pcm_hw_params *params, *sparams; struct snd_pcm_hw_params *params, *sparams;
...@@ -838,11 +838,8 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream, ...@@ -838,11 +838,8 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
const struct snd_mask *sformat_mask; const struct snd_mask *sformat_mask;
struct snd_mask mask; struct snd_mask mask;
if (trylock) { if (!runtime->oss.params)
if (!(mutex_trylock(&runtime->oss.params_lock))) return 0;
return -EAGAIN;
} else if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
sw_params = kzalloc(sizeof(*sw_params), GFP_KERNEL); sw_params = kzalloc(sizeof(*sw_params), GFP_KERNEL);
params = kmalloc(sizeof(*params), GFP_KERNEL); params = kmalloc(sizeof(*params), GFP_KERNEL);
sparams = kmalloc(sizeof(*sparams), GFP_KERNEL); sparams = kmalloc(sizeof(*sparams), GFP_KERNEL);
...@@ -1068,6 +1065,23 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream, ...@@ -1068,6 +1065,23 @@ static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
kfree(sw_params); kfree(sw_params);
kfree(params); kfree(params);
kfree(sparams); kfree(sparams);
return err;
}
/* this one takes the lock by itself */
static int snd_pcm_oss_change_params(struct snd_pcm_substream *substream,
bool trylock)
{
struct snd_pcm_runtime *runtime = substream->runtime;
int err;
if (trylock) {
if (!(mutex_trylock(&runtime->oss.params_lock)))
return -EAGAIN;
} else if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
err = snd_pcm_oss_change_params_locked(substream);
mutex_unlock(&runtime->oss.params_lock); mutex_unlock(&runtime->oss.params_lock);
return err; return err;
} }
...@@ -1096,11 +1110,14 @@ static int snd_pcm_oss_get_active_substream(struct snd_pcm_oss_file *pcm_oss_fil ...@@ -1096,11 +1110,14 @@ static int snd_pcm_oss_get_active_substream(struct snd_pcm_oss_file *pcm_oss_fil
return 0; return 0;
} }
/* call with params_lock held */
static int snd_pcm_oss_prepare(struct snd_pcm_substream *substream) static int snd_pcm_oss_prepare(struct snd_pcm_substream *substream)
{ {
int err; int err;
struct snd_pcm_runtime *runtime = substream->runtime; struct snd_pcm_runtime *runtime = substream->runtime;
if (!runtime->oss.prepare)
return 0;
err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_PREPARE, NULL); err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_PREPARE, NULL);
if (err < 0) { if (err < 0) {
pcm_dbg(substream->pcm, pcm_dbg(substream->pcm,
...@@ -1120,14 +1137,35 @@ static int snd_pcm_oss_make_ready(struct snd_pcm_substream *substream) ...@@ -1120,14 +1137,35 @@ static int snd_pcm_oss_make_ready(struct snd_pcm_substream *substream)
struct snd_pcm_runtime *runtime; struct snd_pcm_runtime *runtime;
int err; int err;
if (substream == NULL)
return 0;
runtime = substream->runtime; runtime = substream->runtime;
if (runtime->oss.params) { if (runtime->oss.params) {
err = snd_pcm_oss_change_params(substream, false); err = snd_pcm_oss_change_params(substream, false);
if (err < 0) if (err < 0)
return err; return err;
} }
if (runtime->oss.prepare) {
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
err = snd_pcm_oss_prepare(substream);
mutex_unlock(&runtime->oss.params_lock);
if (err < 0)
return err;
}
return 0;
}
/* call with params_lock held */
static int snd_pcm_oss_make_ready_locked(struct snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime;
int err;
runtime = substream->runtime;
if (runtime->oss.params) {
err = snd_pcm_oss_change_params_locked(substream);
if (err < 0)
return err;
}
if (runtime->oss.prepare) { if (runtime->oss.prepare) {
err = snd_pcm_oss_prepare(substream); err = snd_pcm_oss_prepare(substream);
if (err < 0) if (err < 0)
...@@ -1332,13 +1370,14 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha ...@@ -1332,13 +1370,14 @@ static ssize_t snd_pcm_oss_write1(struct snd_pcm_substream *substream, const cha
if (atomic_read(&substream->mmap_count)) if (atomic_read(&substream->mmap_count))
return -ENXIO; return -ENXIO;
if ((tmp = snd_pcm_oss_make_ready(substream)) < 0)
return tmp;
while (bytes > 0) { while (bytes > 0) {
if (mutex_lock_interruptible(&runtime->oss.params_lock)) { if (mutex_lock_interruptible(&runtime->oss.params_lock)) {
tmp = -ERESTARTSYS; tmp = -ERESTARTSYS;
break; break;
} }
tmp = snd_pcm_oss_make_ready_locked(substream);
if (tmp < 0)
goto err;
if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) { if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) {
tmp = bytes; tmp = bytes;
if (tmp + runtime->oss.buffer_used > runtime->oss.period_bytes) if (tmp + runtime->oss.buffer_used > runtime->oss.period_bytes)
...@@ -1439,13 +1478,14 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use ...@@ -1439,13 +1478,14 @@ static ssize_t snd_pcm_oss_read1(struct snd_pcm_substream *substream, char __use
if (atomic_read(&substream->mmap_count)) if (atomic_read(&substream->mmap_count))
return -ENXIO; return -ENXIO;
if ((tmp = snd_pcm_oss_make_ready(substream)) < 0)
return tmp;
while (bytes > 0) { while (bytes > 0) {
if (mutex_lock_interruptible(&runtime->oss.params_lock)) { if (mutex_lock_interruptible(&runtime->oss.params_lock)) {
tmp = -ERESTARTSYS; tmp = -ERESTARTSYS;
break; break;
} }
tmp = snd_pcm_oss_make_ready_locked(substream);
if (tmp < 0)
goto err;
if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) { if (bytes < runtime->oss.period_bytes || runtime->oss.buffer_used > 0) {
if (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); tmp = snd_pcm_oss_read2(substream, runtime->oss.buffer, runtime->oss.period_bytes, 1);
...@@ -1501,10 +1541,12 @@ static int snd_pcm_oss_reset(struct snd_pcm_oss_file *pcm_oss_file) ...@@ -1501,10 +1541,12 @@ static int snd_pcm_oss_reset(struct snd_pcm_oss_file *pcm_oss_file)
continue; continue;
runtime = substream->runtime; runtime = substream->runtime;
snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL); snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
mutex_lock(&runtime->oss.params_lock);
runtime->oss.prepare = 1; runtime->oss.prepare = 1;
runtime->oss.buffer_used = 0; runtime->oss.buffer_used = 0;
runtime->oss.prev_hw_ptr_period = 0; runtime->oss.prev_hw_ptr_period = 0;
runtime->oss.period_ptr = 0; runtime->oss.period_ptr = 0;
mutex_unlock(&runtime->oss.params_lock);
} }
return 0; return 0;
} }
...@@ -1590,9 +1632,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) ...@@ -1590,9 +1632,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
goto __direct; goto __direct;
if ((err = snd_pcm_oss_make_ready(substream)) < 0) if ((err = snd_pcm_oss_make_ready(substream)) < 0)
return err; return err;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
format = snd_pcm_oss_format_from(runtime->oss.format); format = snd_pcm_oss_format_from(runtime->oss.format);
width = snd_pcm_format_physical_width(format); width = snd_pcm_format_physical_width(format);
mutex_lock(&runtime->oss.params_lock);
if (runtime->oss.buffer_used > 0) { if (runtime->oss.buffer_used > 0) {
#ifdef OSS_DEBUG #ifdef OSS_DEBUG
pcm_dbg(substream->pcm, "sync: buffer_used\n"); pcm_dbg(substream->pcm, "sync: buffer_used\n");
...@@ -1643,7 +1686,9 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) ...@@ -1643,7 +1686,9 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
substream->f_flags = saved_f_flags; substream->f_flags = saved_f_flags;
if (err < 0) if (err < 0)
return err; return err;
mutex_lock(&runtime->oss.params_lock);
runtime->oss.prepare = 1; runtime->oss.prepare = 1;
mutex_unlock(&runtime->oss.params_lock);
} }
substream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE]; substream = pcm_oss_file->streams[SNDRV_PCM_STREAM_CAPTURE];
...@@ -1654,8 +1699,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) ...@@ -1654,8 +1699,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL); err = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL);
if (err < 0) if (err < 0)
return err; return err;
mutex_lock(&runtime->oss.params_lock);
runtime->oss.buffer_used = 0; runtime->oss.buffer_used = 0;
runtime->oss.prepare = 1; runtime->oss.prepare = 1;
mutex_unlock(&runtime->oss.params_lock);
} }
return 0; return 0;
} }
...@@ -1674,10 +1721,13 @@ static int snd_pcm_oss_set_rate(struct snd_pcm_oss_file *pcm_oss_file, int rate) ...@@ -1674,10 +1721,13 @@ static int snd_pcm_oss_set_rate(struct snd_pcm_oss_file *pcm_oss_file, int rate)
rate = 1000; rate = 1000;
else if (rate > 192000) else if (rate > 192000)
rate = 192000; rate = 192000;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
if (runtime->oss.rate != rate) { if (runtime->oss.rate != rate) {
runtime->oss.params = 1; runtime->oss.params = 1;
runtime->oss.rate = rate; runtime->oss.rate = rate;
} }
mutex_unlock(&runtime->oss.params_lock);
} }
return snd_pcm_oss_get_rate(pcm_oss_file); return snd_pcm_oss_get_rate(pcm_oss_file);
} }
...@@ -1705,10 +1755,13 @@ static int snd_pcm_oss_set_channels(struct snd_pcm_oss_file *pcm_oss_file, unsig ...@@ -1705,10 +1755,13 @@ static int snd_pcm_oss_set_channels(struct snd_pcm_oss_file *pcm_oss_file, unsig
if (substream == NULL) if (substream == NULL)
continue; continue;
runtime = substream->runtime; runtime = substream->runtime;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
if (runtime->oss.channels != channels) { if (runtime->oss.channels != channels) {
runtime->oss.params = 1; runtime->oss.params = 1;
runtime->oss.channels = channels; runtime->oss.channels = channels;
} }
mutex_unlock(&runtime->oss.params_lock);
} }
return snd_pcm_oss_get_channels(pcm_oss_file); return snd_pcm_oss_get_channels(pcm_oss_file);
} }
...@@ -1794,10 +1847,13 @@ static int snd_pcm_oss_set_format(struct snd_pcm_oss_file *pcm_oss_file, int for ...@@ -1794,10 +1847,13 @@ static int snd_pcm_oss_set_format(struct snd_pcm_oss_file *pcm_oss_file, int for
if (substream == NULL) if (substream == NULL)
continue; continue;
runtime = substream->runtime; runtime = substream->runtime;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
if (runtime->oss.format != format) { if (runtime->oss.format != format) {
runtime->oss.params = 1; runtime->oss.params = 1;
runtime->oss.format = format; runtime->oss.format = format;
} }
mutex_unlock(&runtime->oss.params_lock);
} }
} }
return snd_pcm_oss_get_format(pcm_oss_file); return snd_pcm_oss_get_format(pcm_oss_file);
...@@ -1817,8 +1873,6 @@ static int snd_pcm_oss_set_subdivide1(struct snd_pcm_substream *substream, int s ...@@ -1817,8 +1873,6 @@ static int snd_pcm_oss_set_subdivide1(struct snd_pcm_substream *substream, int s
{ {
struct snd_pcm_runtime *runtime; struct snd_pcm_runtime *runtime;
if (substream == NULL)
return 0;
runtime = substream->runtime; runtime = substream->runtime;
if (subdivide == 0) { if (subdivide == 0) {
subdivide = runtime->oss.subdivision; subdivide = runtime->oss.subdivision;
...@@ -1842,9 +1896,16 @@ static int snd_pcm_oss_set_subdivide(struct snd_pcm_oss_file *pcm_oss_file, int ...@@ -1842,9 +1896,16 @@ static int snd_pcm_oss_set_subdivide(struct snd_pcm_oss_file *pcm_oss_file, int
for (idx = 1; idx >= 0; --idx) { for (idx = 1; idx >= 0; --idx) {
struct snd_pcm_substream *substream = pcm_oss_file->streams[idx]; struct snd_pcm_substream *substream = pcm_oss_file->streams[idx];
struct snd_pcm_runtime *runtime;
if (substream == NULL) if (substream == NULL)
continue; continue;
if ((err = snd_pcm_oss_set_subdivide1(substream, subdivide)) < 0) runtime = substream->runtime;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
err = snd_pcm_oss_set_subdivide1(substream, subdivide);
mutex_unlock(&runtime->oss.params_lock);
if (err < 0)
return err; return err;
} }
return err; return err;
...@@ -1854,8 +1915,6 @@ static int snd_pcm_oss_set_fragment1(struct snd_pcm_substream *substream, unsign ...@@ -1854,8 +1915,6 @@ static int snd_pcm_oss_set_fragment1(struct snd_pcm_substream *substream, unsign
{ {
struct snd_pcm_runtime *runtime; struct snd_pcm_runtime *runtime;
if (substream == NULL)
return 0;
runtime = substream->runtime; runtime = substream->runtime;
if (runtime->oss.subdivision || runtime->oss.fragshift) if (runtime->oss.subdivision || runtime->oss.fragshift)
return -EINVAL; return -EINVAL;
...@@ -1875,9 +1934,16 @@ static int snd_pcm_oss_set_fragment(struct snd_pcm_oss_file *pcm_oss_file, unsig ...@@ -1875,9 +1934,16 @@ static int snd_pcm_oss_set_fragment(struct snd_pcm_oss_file *pcm_oss_file, unsig
for (idx = 1; idx >= 0; --idx) { for (idx = 1; idx >= 0; --idx) {
struct snd_pcm_substream *substream = pcm_oss_file->streams[idx]; struct snd_pcm_substream *substream = pcm_oss_file->streams[idx];
struct snd_pcm_runtime *runtime;
if (substream == NULL) if (substream == NULL)
continue; continue;
if ((err = snd_pcm_oss_set_fragment1(substream, val)) < 0) runtime = substream->runtime;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
err = snd_pcm_oss_set_fragment1(substream, val);
mutex_unlock(&runtime->oss.params_lock);
if (err < 0)
return err; return err;
} }
return err; return err;
...@@ -1961,6 +2027,9 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr ...@@ -1961,6 +2027,9 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
} }
if (psubstream) { if (psubstream) {
runtime = psubstream->runtime; runtime = psubstream->runtime;
cmd = 0;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
if (trigger & PCM_ENABLE_OUTPUT) { if (trigger & PCM_ENABLE_OUTPUT) {
if (runtime->oss.trigger) if (runtime->oss.trigger)
goto _skip1; goto _skip1;
...@@ -1978,13 +2047,19 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr ...@@ -1978,13 +2047,19 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
cmd = SNDRV_PCM_IOCTL_DROP; cmd = SNDRV_PCM_IOCTL_DROP;
runtime->oss.prepare = 1; runtime->oss.prepare = 1;
} }
_skip1:
mutex_unlock(&runtime->oss.params_lock);
if (cmd) {
err = snd_pcm_kernel_ioctl(psubstream, cmd, NULL); err = snd_pcm_kernel_ioctl(psubstream, cmd, NULL);
if (err < 0) if (err < 0)
return err; return err;
} }
_skip1: }
if (csubstream) { if (csubstream) {
runtime = csubstream->runtime; runtime = csubstream->runtime;
cmd = 0;
if (mutex_lock_interruptible(&runtime->oss.params_lock))
return -ERESTARTSYS;
if (trigger & PCM_ENABLE_INPUT) { if (trigger & PCM_ENABLE_INPUT) {
if (runtime->oss.trigger) if (runtime->oss.trigger)
goto _skip2; goto _skip2;
...@@ -1999,11 +2074,14 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr ...@@ -1999,11 +2074,14 @@ static int snd_pcm_oss_set_trigger(struct snd_pcm_oss_file *pcm_oss_file, int tr
cmd = SNDRV_PCM_IOCTL_DROP; cmd = SNDRV_PCM_IOCTL_DROP;
runtime->oss.prepare = 1; runtime->oss.prepare = 1;
} }
_skip2:
mutex_unlock(&runtime->oss.params_lock);
if (cmd) {
err = snd_pcm_kernel_ioctl(csubstream, cmd, NULL); err = snd_pcm_kernel_ioctl(csubstream, cmd, NULL);
if (err < 0) if (err < 0)
return err; return err;
} }
_skip2: }
return 0; return 0;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7