summaryrefslogtreecommitdiffstats
path: root/sound/core
diff options
context:
space:
mode:
authorTakashi Iwai <tiwai@suse.de>2017-06-16 22:29:55 +0200
committerTakashi Iwai <tiwai@suse.de>2017-06-20 07:55:59 +0200
commitaa30db060121f688d01f74f8d3fe603f7c4c731c (patch)
tree394bf468d0fe87aae513a108bc60caec7413dde0 /sound/core
parenteb8d0eaaf84b0398533a7c091a0b65663f2fd7ea (diff)
downloadlinux-aa30db060121f688d01f74f8d3fe603f7c4c731c.tar.bz2
ALSA: pcm: Fix possible inconsistent appl_ptr update via mmap
The ALSA PCM core refers to the appl_ptr value stored on the mmapped page that is shared between kernel and user-space. Although the reference is performed in the PCM stream lock, it doesn't guarantee the atomic access when the value gets updated concurrently from the user-space on another CPU. In most of codes, this is no big problem, but still there are a few places that may result in slight inconsistencies because they access runtime->control->appl_ptr multiple times; that is, the second read might be a different value from the first value. It can be even backward or jumping, as we have no control for it. Hence, the calculation may give an unexpected value. Luckily, there is no security vulnerability by that, as far as I've checked. But still we should address it. This patch tries to reduce such possible cases. The fix is simple -- we just read once, store it to a local variable and use it for the rest calculations. The READ_ONCE() macro is used for it in order to avoid the ill-effect by possible compiler optimizations. Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Diffstat (limited to 'sound/core')
-rw-r--r--sound/core/pcm_lib.c13
1 files changed, 7 insertions, 6 deletions
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index e8131c060c86..e76d55a4d1b2 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -65,15 +65,16 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
if (runtime->silence_size < runtime->boundary) {
snd_pcm_sframes_t noise_dist, n;
- if (runtime->silence_start != runtime->control->appl_ptr) {
- n = runtime->control->appl_ptr - runtime->silence_start;
+ snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
+ if (runtime->silence_start != appl_ptr) {
+ n = appl_ptr - runtime->silence_start;
if (n < 0)
n += runtime->boundary;
if ((snd_pcm_uframes_t)n < runtime->silence_filled)
runtime->silence_filled -= n;
else
runtime->silence_filled = 0;
- runtime->silence_start = runtime->control->appl_ptr;
+ runtime->silence_start = appl_ptr;
}
if (runtime->silence_filled >= runtime->buffer_size)
return;
@@ -2203,7 +2204,9 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
continue; /* draining */
}
frames = size > avail ? avail : size;
- cont = runtime->buffer_size - runtime->control->appl_ptr % runtime->buffer_size;
+ appl_ptr = READ_ONCE(runtime->control->appl_ptr);
+ appl_ofs = appl_ptr % runtime->buffer_size;
+ cont = runtime->buffer_size - appl_ofs;
if (frames > cont)
frames = cont;
if (snd_BUG_ON(!frames)) {
@@ -2211,8 +2214,6 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
snd_pcm_stream_unlock_irq(substream);
return -EINVAL;
}
- appl_ptr = runtime->control->appl_ptr;
- appl_ofs = appl_ptr % runtime->buffer_size;
snd_pcm_stream_unlock_irq(substream);
err = writer(substream, appl_ofs, data, offset, frames,
transfer);