Skip to contentSkip to conversation

ALSA: firewire-tascam: off by one in identify_model()

PATCH Superseded Dan Carpenter 3 participants 7 messages Oct 15, 2015 – Oct 19, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ALSA: firewire-tascam: off by one in identify_model()

In the original code, we potentially put a NUL character in model[8] and it caused a static checker warning. We can put the NUL in model[7] instead.

Fixes 53b3ffee7885 ('ALSA: firewire-tascam: change device probing processing')
sound/firewire/tascam/tascam.c +1 -1
index c6747a4..6ef2bcc 100644
@@ -51,7 +51,7 @@ static int identify_model(struct snd_tscm *tscm)
 	}
 
 	/* Pick up model name from certain addresses. */
-	for (i = 0; i < 8; i++) {
+	for (i = 0; i < 7; i++) {
 		c = config_rom[28 + i / 4] >> (24 - 8 * (i % 4));
 		if (c == '\0')
 			break;
#1 lore raw
Quoted 4 lines · Dan Carpenter

In the original code, we potentially put a NUL character in model[8] and it caused a static checker warning. We can put the NUL in model[7] instead.

I guess it's better to expand model[] to 9 bytes instead. The logic is to retrieve the string from the config rom, and it's up to 8 letters.

thanks,

Takashi

Quoted 18 lines
Fixes 53b3ffee7885 ('ALSA: firewire-tascam: change device probing processing')
sound/firewire/tascam/tascam.c +1 -1
index c6747a4..6ef2bcc 100644
@@ -51,7 +51,7 @@ static int identify_model(struct snd_tscm *tscm)
 	}
 
 	/* Pick up model name from certain addresses. */
-	for (i = 0; i < 8; i++) {
+	for (i = 0; i < 7; i++) {
 		c = config_rom[28 + i / 4] >> (24 - 8 * (i % 4));
 		if (c == '\0')
 			break;
#2 lore raw
Quoted 10 lines · Takashi Iwai
Quoted 4 lines · Dan Carpenter

In the original code, we potentially put a NUL character in model[8] and it caused a static checker warning. We can put the NUL in model[7] instead.

I guess it's better to expand model[] to 9 bytes instead. The logic is to retrieve the string from the config rom, and it's up to 8 letters.

Ok. I will resend.

regards,
dan carpenter

maintainer
#3 lore raw
Quoted 8 lines · Dan Carpenter
Quoted 7 lines · Takashi Iwai
Quoted 4 lines · Dan Carpenter

In the original code, we potentially put a NUL character in model[8] and it caused a static checker warning. We can put the NUL in model[7] instead.

Quoted 5 lines
Quoted 3 lines

I guess it's better to expand model[] to 9 bytes instead. The logic is to retrieve the string from the config rom, and it's up to 8 letters.

Ok. I will resend.

I think this idea is better, too.

Thanks

Takashi Sakamoto

#4 lore raw

PATCH v2 1/1 ALSA: firewire-tascam: off by one in identify_model()

Let's leave space for the NUL char otherwise the static checkers complain that we go beyond the end of the array.

Fixes 53b3ffee7885 ('ALSA: firewire-tascam: change device probing processing')

---
v2: I truncated the last char in v1 instead of making the buffer larger

sound/firewire/tascam/tascam.c +1 -1
index c6747a4..79cb9e9 100644
@@ -40,7 +40,7 @@ static int identify_model(struct snd_tscm *tscm)
 {
 	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
 	const u32 *config_rom = fw_dev->config_rom;
-	char model[8];
+	char model[9];
 	unsigned int i;
 	u8 c;
 
maintainer Reviewed-by on v2 1/1
#5 lore raw
Quoted 5 lines · Dan Carpenter

Let's leave space for the NUL char otherwise the static checkers complain that we go beyond the end of the array.

Fixes 53b3ffee7885 ('ALSA: firewire-tascam: change device probing processing')

Thank you to fix it.

Takashi Sakamoto

Quoted 17 lines

---
v2: I truncated the last char in v1 instead of making the buffer larger

sound/firewire/tascam/tascam.c +1 -1
index c6747a4..79cb9e9 100644
@@ -40,7 +40,7 @@ static int identify_model(struct snd_tscm *tscm)
 {
 	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
 	const u32 *config_rom = fw_dev->config_rom;
-	char model[8];
+	char model[9];
 	unsigned int i;
 	u8 c;
 
Applied
#6 lore raw
In reply to Dan Carpenter
Quoted 8 lines · Dan Carpenter

Let's leave space for the NUL char otherwise the static checkers complain that we go beyond the end of the array.

Fixes 53b3ffee7885 ('ALSA: firewire-tascam: change device probing processing')

---
v2: I truncated the last char in v1 instead of making the buffer larger

Applied, thanks.

Takashi

Quoted 15 lines
sound/firewire/tascam/tascam.c +1 -1
index c6747a4..79cb9e9 100644
@@ -40,7 +40,7 @@ static int identify_model(struct snd_tscm *tscm)
 {
 	struct fw_device *fw_dev = fw_parent_device(tscm->unit);
 	const u32 *config_rom = fw_dev->config_rom;
-	char model[8];
+	char model[9];
 	unsigned int i;
 	u8 c;