alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [alsa-devel] [PATCH] ALSA: pcm: Fill silence on buffers at hw_params
@ 2019-12-10 13:02 Takashi Iwai
  2019-12-10 17:25 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Takashi Iwai @ 2019-12-10 13:02 UTC (permalink / raw)
  To: alsa-devel; +Cc: Lionel Koenig

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@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 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@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: pcm: Fill silence on buffers at hw_params
  2019-12-10 13:02 [alsa-devel] [PATCH] ALSA: pcm: Fill silence on buffers at hw_params Takashi Iwai
@ 2019-12-10 17:25 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2019-12-10 17:25 UTC (permalink / raw)
  To: alsa-devel; +Cc: Lionel Koenig

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@gmail.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

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@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-10 17:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 13:02 [alsa-devel] [PATCH] ALSA: pcm: Fill silence on buffers at hw_params Takashi Iwai
2019-12-10 17:25 ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).