[alsa-devel] [PATCH] ALSA: pcm: Fix possible inconsistent appl_ptr update via mmap

[Clemens Ladisch]

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