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

[Takashi Iwai]

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.


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++;
 		}