Takashi Iwai wrote:
> 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.
It is likely that the compiler's optimizer already merged multiple
accesses and used a temporary register.
However, if the compiler can do this transformation, it can also do the
reverse transformation. So this new code does not actually guarantee
that there is a single atomic access. That can be enforced only with
READ_ONCE():
> 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 = runtime->control->appl_ptr;
snd_pcm_uframes_t appl_ptr = READ_ONCE(runtime->control->appl_ptr);
etc.
Regards,
Clemens
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
