On Tue, 10 Dec 2019 14:02:19 +0100,
Takashi Iwai wrote:
>
> The current PCM code doesn't initialize explicitly the buffers
> allocated for PCM streams, hence it might leak some uninitialized
> kernel data by mmapping or reading the buffer before actually reading
> or writing.
>
> Since this is a common problem, let's initialize the data on the
> buffers each hw_params properly. For that, an existing helper
> function is exposed as snd_pcm_fill_silence_frames() and call it from
> snd_pcm_hw_params().
>
> Reported-and-tested-by: Lionel Koenig <lionel.koenig@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
On the second thought, this use of fill_silence ops might have some
side-effect on the drivers that do actually transfer stream data by
that ops. So I dropped this patch again.
Will submit the revised patch later. Sorry for the trouble.
Takashi
> ---
> sound/core/pcm_lib.c | 13 ++++---------
> sound/core/pcm_local.h | 2 ++
> sound/core/pcm_native.c | 3 +++
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 2236b5e0c1f2..3c63324b8bb7 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -30,9 +30,6 @@
> #define trace_applptr(substream, prev, curr)
> #endif
>
> -static int fill_silence_frames(struct snd_pcm_substream *substream,
> - snd_pcm_uframes_t off, snd_pcm_uframes_t frames);
> -
> /*
> * fill ring buffer with silence
> * runtime->silence_start: starting pointer to silence area
> @@ -100,7 +97,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram
> ofs = runtime->silence_start % runtime->buffer_size;
> while (frames > 0) {
> transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames;
> - err = fill_silence_frames(substream, ofs, transfer);
> + err = snd_pcm_fill_silence_frames(substream, ofs, transfer);
> snd_BUG_ON(err < 0);
> runtime->silence_filled += transfer;
> frames -= transfer;
> @@ -1945,8 +1942,6 @@ static int fill_silence(struct snd_pcm_substream *substream, int channel,
> {
> struct snd_pcm_runtime *runtime = substream->runtime;
>
> - if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK)
> - return 0;
> if (substream->ops->fill_silence)
> return substream->ops->fill_silence(substream, channel,
> hwoff, bytes);
> @@ -2030,10 +2025,10 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream,
> }
>
> /* fill silence on the given buffer position;
> - * called from snd_pcm_playback_silence()
> + * called from snd_pcm_playback_silence() and snd_pcm_hw_params()
> */
> -static int fill_silence_frames(struct snd_pcm_substream *substream,
> - snd_pcm_uframes_t off, snd_pcm_uframes_t frames)
> +int snd_pcm_fill_silence_frames(struct snd_pcm_substream *substream,
> + snd_pcm_uframes_t off, snd_pcm_uframes_t frames)
> {
> if (substream->runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED ||
> substream->runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED)
> diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h
> index 384efd002984..ac4f455b1fff 100644
> --- a/sound/core/pcm_local.h
> +++ b/sound/core/pcm_local.h
> @@ -34,6 +34,8 @@ int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream);
>
> void snd_pcm_playback_silence(struct snd_pcm_substream *substream,
> snd_pcm_uframes_t new_hw_ptr);
> +int snd_pcm_fill_silence_frames(struct snd_pcm_substream *substream,
> + snd_pcm_uframes_t off, snd_pcm_uframes_t frames);
>
> static inline snd_pcm_uframes_t
> snd_pcm_avail(struct snd_pcm_substream *substream)
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 1fe581167b7b..d6f270c639b4 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -739,6 +739,9 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
> while (runtime->boundary * 2 <= LONG_MAX - runtime->buffer_size)
> runtime->boundary *= 2;
>
> + /* clear the buffer once for avoiding information leaks */
> + snd_pcm_fill_silence_frames(substream, 0, runtime->period_size);
> +
> snd_pcm_timer_resolution_change(substream);
> snd_pcm_set_state(substream, SNDRV_PCM_STATE_SETUP);
>
> --
> 2.16.4
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
