Re: [alsa-devel] ALSA: Add ALSA driver for Atmel Audio Bitstream DAC
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 }
regards, dan carpenter
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++; }
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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Dan Carpenter -
Takashi Iwai