Re: [smatch stuff] reading beyond the end of the array in build_feature_ctl()

[Takashi Iwai <tiwai@suse.de>]

At Thu, 6 Oct 2011 16:39:48 +0300,
Dan Carpenter wrote:
> 
> Hi Daniel,
> 
> Smatch complains about a potential read past the end of the array in
> build_feature_ctl().
> 
> sound/usb/mixer.c +1035 build_feature_ctl(34)
> 	error: buffer overflow 'audio_feature_info' 13 <= 14
> 
> This is in the init code so probably it's correct, and I've just
> missed how it works.  Can you take a look?
> 
> The problem is that audio_feature_info[] has 13 elements, but Smatch
> thinks control - 1 can go up to 14 so we're two elements past the end.
> 
>   1035          cval->val_type = audio_feature_info[control-1].type;
> 
> Smatch get the value for "control" from "i" in parse_audio_feature_unit().

It looks like a flaw, indeed.
audio_feature_info[] should contain all data for UAC2_FU_*.

An easy fix is a patch like below.
(It's anyway better to replace the digits there.)
Later we can add the missing entries to audio_feature_info[].


thanks,

Takashi

---
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index b13b7ac..60f65ac 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1259,7 +1259,7 @@ static int parse_audio_feature_unit(struct mixer_build *state, int unitid, void
 				build_feature_ctl(state, _ftr, 0, i, &iterm, unitid, 0);
 		}
 	} else { /* UAC_VERSION_2 */
-		for (i = 0; i < 30/2; i++) {
+		for (i = 0; i < ARRAY_SIZE(audio_feature_info); i++) {
 			unsigned int ch_bits = 0;
 			unsigned int ch_read_only = 0;