All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Colin King <colin.king@canonical.com>
Cc: alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.com>,
	Alexander Tsoy <alexander@tsoy.me>
Subject: Re: [PATCH][next] ALSA: usb-audio: Fix sum of uninitialized variable sample_accum
Date: Fri, 01 Oct 2021 12:48:29 +0200	[thread overview]
Message-ID: <s5htui1hvgi.wl-tiwai@suse.de> (raw)
In-Reply-To: <20211001104417.14291-1-colin.king@canonical.com>

On Fri, 01 Oct 2021 12:44:17 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable sample_accum is not being intialized and then has
> ep->sample_rem added to it, leading to a bogus value. One solution
> is to initialize it to zero at declaration time, but it is probably
> best to just assign it to ep->sample_rem on first use.
> 
> Addresses-Coveriry: ("Uninitialized scalar variable")
> Fixes: f0bd62b64016 ("ALSA: usb-audio: Improve frames size computation")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Thanks for the patch, but it's no right fix.  The Fixes tag points to
a wrong commit, it was d215f63d49da9a8803af3e81acd6cad743686573
    ALSA: usb-audio: Check available frames for the next packet size
  
And sample_accum has to be initialized from ep->sample_accum instead.
I'll post the proper fix.


Takashi


> ---
>  sound/usb/endpoint.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 42c0d2db8ba8..c6a33732db3f 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -182,7 +182,7 @@ static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail)
>  	if (ep->fill_max)
>  		return ep->maxframesize;
>  
> -	sample_accum += ep->sample_rem;
> +	sample_accum = ep->sample_rem;
>  	if (sample_accum >= ep->pps) {
>  		sample_accum -= ep->pps;
>  		ret = ep->packsize[1];
> -- 
> 2.32.0
> 

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Colin King <colin.king@canonical.com>
Cc: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Alexander Tsoy <alexander@tsoy.me>,
	alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] ALSA: usb-audio: Fix sum of uninitialized variable sample_accum
Date: Fri, 01 Oct 2021 12:48:29 +0200	[thread overview]
Message-ID: <s5htui1hvgi.wl-tiwai@suse.de> (raw)
In-Reply-To: <20211001104417.14291-1-colin.king@canonical.com>

On Fri, 01 Oct 2021 12:44:17 +0200,
Colin King wrote:
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable sample_accum is not being intialized and then has
> ep->sample_rem added to it, leading to a bogus value. One solution
> is to initialize it to zero at declaration time, but it is probably
> best to just assign it to ep->sample_rem on first use.
> 
> Addresses-Coveriry: ("Uninitialized scalar variable")
> Fixes: f0bd62b64016 ("ALSA: usb-audio: Improve frames size computation")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Thanks for the patch, but it's no right fix.  The Fixes tag points to
a wrong commit, it was d215f63d49da9a8803af3e81acd6cad743686573
    ALSA: usb-audio: Check available frames for the next packet size
  
And sample_accum has to be initialized from ep->sample_accum instead.
I'll post the proper fix.


Takashi


> ---
>  sound/usb/endpoint.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
> index 42c0d2db8ba8..c6a33732db3f 100644
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -182,7 +182,7 @@ static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail)
>  	if (ep->fill_max)
>  		return ep->maxframesize;
>  
> -	sample_accum += ep->sample_rem;
> +	sample_accum = ep->sample_rem;
>  	if (sample_accum >= ep->pps) {
>  		sample_accum -= ep->pps;
>  		ret = ep->packsize[1];
> -- 
> 2.32.0
> 

  reply	other threads:[~2021-10-01 10:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 10:44 [PATCH][next] ALSA: usb-audio: Fix sum of uninitialized variable sample_accum Colin King
2021-10-01 10:48 ` Takashi Iwai [this message]
2021-10-01 10:48   ` Takashi Iwai
2021-10-01 11:07   ` Colin Ian King
2021-10-01 11:07     ` Colin Ian King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5htui1hvgi.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alexander@tsoy.me \
    --cc=alsa-devel@alsa-project.org \
    --cc=colin.king@canonical.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.