[alsa-devel] ALSA: Add ALSA driver for Atmel Audio Bitstream DAC

[Takashi Iwai]

At Fri, 29 Nov 2013 10:25:41 +0100,
Takashi Iwai wrote:
> 
> At Wed, 27 Nov 2013 14:57:32 +0300,
> Dan Carpenter wrote:
> > 
> > Hello Hans-Christian Egtvedt,
> > 
> > The patch e4967d6016b7: "ALSA: Add ALSA driver for Atmel Audio 
> > Bitstream DAC" from Feb 5, 2009, leads to the following
> > static checker warning: "sound/atmel/abdac.c:373 set_sample_rates()
> > 	 error: buffer overflow 'dac->rates' 6 <= 6"
> > 
> > sound/atmel/abdac.c
> >    354          /* we start at 192 kHz and work our way down to 5112 Hz */
> >    355          while (new_rate >= RATE_MIN && index < (MAX_NUM_RATES + 1)) {
> > 
> > index == MAX_NUM_RATES + 1 so index is 7.
> > 
> >    356                  new_rate = clk_round_rate(dac->sample_clk, 256 * new_rate);
> >    357                  if (new_rate < 0)
> >    358                          break;
> >    359                  /* make sure we are below the ABDAC clock */
> >    360                  if (new_rate <= clk_get_rate(dac->pclk)) {
> >    361                          dac->rates[index] = new_rate / 256;
> > 
> > index == MAX_NUM_RATES is off by one.
> > 
> >    362                          index++;
> >    363                  }
> >    364                  /* divide by 256 and then by two to get next rate */
> >    365                  new_rate /= 256 * 2;
> >    366          }
> >    367  
> >    368          if (index) {
> >    369                  int i;
> >    370  
> >    371                  /* reverse array, smallest go first */
> >    372                  for (i = 0; i < (index / 2); i++) {
> >    373                          unsigned int tmp = dac->rates[index - 1 - i];
> > 
> > 7 - 1 - 0 is 6, but dac->rates[] only has 6 elements so we are
> > potentially reading beyond the end of the array here.
> > 
> >    374                          dac->rates[index - 1 - i] = dac->rates[i];
> >    375                          dac->rates[i] = tmp;
> >    376                  }
> >    377  
> >    378                  dac->constraints_rates.count = index;
> >    379                  dac->constraints_rates.list = dac->rates;
> >    380                  dac->constraints_rates.mask = 0;
> >    381                  dac->rates_num = index;
> >    382  
> >    383                  retval = 0;
> >    384          }
> 
> I guess the patch below should fix, but better to hear from
> Hans-Christian at first.

It seems that the post isn't reachable to him.

Then, I'm going to merge the fix later if no one objects.


Takashi


> 
> 
> thanks,
> 
> Takashi
> 
> ---
> diff --git a/sound/atmel/abdac.c b/sound/atmel/abdac.c
> index 872d59e35ee2..721d8fd45685 100644
> --- a/sound/atmel/abdac.c
> +++ b/sound/atmel/abdac.c
> @@ -357,7 +357,8 @@ static int set_sample_rates(struct atmel_abdac *dac)
>  		if (new_rate < 0)
>  			break;
>  		/* make sure we are below the ABDAC clock */
> -		if (new_rate <= clk_get_rate(dac->pclk)) {
> +		if (index < MAX_NUM_RATES &&
> +		    new_rate <= clk_get_rate(dac->pclk)) {
>  			dac->rates[index] = new_rate / 256;
>  			index++;
>  		}
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>