linux-kernel - Re: [PATCH] ALSA: pcm: fix incorrect hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 May 2020 09:21:10 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Brent Lu <brent.lu@...el.com>
Cc:     <alsa-devel@...a-project.org>, Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Baolin Wang <baolin.wang@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Richard Fontana <rfontana@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        paulhsia <paulhsia@...omium.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ALSA: pcm: fix incorrect hw_base increase

On Fri, 15 May 2020 06:09:39 +0200,
Brent Lu wrote:
> 
> The hw_base will be increased by runtime->buffer_size frames
> unconditionally if the runtime->status->hw_ptr is not updated for over
> half of buffer time. As the hw_base increases, so does the
> runtime->status->hw_ptr which could lead to invalid return value when
> user space program calls snd_pcm_avail() function.
> 
> By updating runtime->hw_ptr_jiffies each time the HWSYNC is called,
> the hw_base will keep the same when buffer stall happens, so does the
> hw_ptr.

Updating hw_ptr_jiffies at that code path looks correct, but it still
leaves the question why this condition happens.  It means that the
actual hwptr isn't changed and yet only jiffies increase
significantly; it means that the hardware can't report proper pointer,
and it should have set SNDRV_PCM_INFO_BATCH flag, then the jiffies
check is skipped.

With which hardware and under which situation did it happen (and the
patch fixed)?


thanks,

Takashi

> 
> Signed-off-by: Brent Lu <brent.lu@...el.com>
> ---
>  sound/core/pcm_lib.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 872a852..d531e1b 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -433,6 +433,7 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
>  
>   no_delta_check:
>  	if (runtime->status->hw_ptr == new_hw_ptr) {
> +		runtime->hw_ptr_jiffies = curr_jiffies;
>  		update_audio_tstamp(substream, &curr_tstamp, &audio_tstamp);
>  		return 0;
>  	}
> -- 
> 2.7.4
>