Skip to contentSkip to conversation

ALSA: hda - potential (but unlikely) uninitialized variable

PATCH Superseded Dan Carpenter 2 participants 5 messages Apr 17, 2015

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

PATCH 1/1 ALSA: hda - potential (but unlikely) uninitialized variable

This function is a bit unusual because it accepts negative values as "conn_len". It's theoretically possible for both "cache_len" and "conn_len" to be -ENOSPC and in that case we would oops trying to run memcmp() on the uninitialized "list" pointer.

My static checker complains about this.

sound/pci/hda/hda_proc.c +2 -2
index ce5a6da..387fdfc 100644
@@ -556,8 +556,8 @@ static void print_conn_list(struct snd_info_buffer *buffer,
 
 	/* Get Cache connections info */
 	cache_len = snd_hda_get_conn_list(codec, nid, &list);
-	if (cache_len != conn_len
-			|| memcmp(list, conn, conn_len)) {
+	if (cache_len < 0 || cache_len != conn_len ||
+	    memcmp(list, conn, conn_len) != 0) {
 		snd_iprintf(buffer, "  In-driver Connection: %d\n", cache_len);
 		if (cache_len > 0) {
 			snd_iprintf(buffer, "    ");
#1 lore raw

At Fri, 17 Apr 2015 15:35:15 +0300,

Quoted 5 lines · Dan Carpenter

This function is a bit unusual because it accepts negative values as "conn_len". It's theoretically possible for both "cache_len" and "conn_len" to be -ENOSPC and in that case we would oops trying to run memcmp() on the uninitialized "list" pointer.

Yes, that's a bug. But the check should be rather:

	if (cache_len >= 0 && (cache_len != conn_len ||
		    memcmp(list, conn, conn_len) != 0))

Could you resend with this fix?

thanks,

Takashi

Quoted 21 lines

My static checker complains about this.

sound/pci/hda/hda_proc.c +2 -2
index ce5a6da..387fdfc 100644
@@ -556,8 +556,8 @@ static void print_conn_list(struct snd_info_buffer *buffer,
 
 	/* Get Cache connections info */
 	cache_len = snd_hda_get_conn_list(codec, nid, &list);
-	if (cache_len != conn_len
-			|| memcmp(list, conn, conn_len)) {
+	if (cache_len < 0 || cache_len != conn_len ||
+	    memcmp(list, conn, conn_len) != 0) {
 		snd_iprintf(buffer, "  In-driver Connection: %d\n", cache_len);
 		if (cache_len > 0) {
 			snd_iprintf(buffer, "    ");
#2 lore raw
Quoted 8 lines · Takashi Iwai

At Fri, 17 Apr 2015 15:35:15 +0300,

Quoted 5 lines · Dan Carpenter

This function is a bit unusual because it accepts negative values as "conn_len". It's theoretically possible for both "cache_len" and "conn_len" to be -ENOSPC and in that case we would oops trying to run memcmp() on the uninitialized "list" pointer.

Quoted 6 lines

Yes, that's a bug. But the check should be rather:

	if (cache_len >= 0 && (cache_len != conn_len ||
		    memcmp(list, conn, conn_len) != 0))

Could you resend with this fix?

Sure. Will do.

regards,
dan carpenter

#3 lore raw

PATCH v2 1/1 ALSA: hda - potential (but unlikely) uninitialized variable

In reply to Takashi Iwai

This function is a bit unusual because it accepts negative values as "conn_len". It's theoretically possible for both "cache_len" and "conn_len" to be -ENOSPC and in that case we would oops trying to run memcmp() on the uninitialized "list" pointer.

---
v2: Don't print a negative error code as "In-driver Connection."

sound/pci/hda/hda_proc.c +2 -2
index ee62307..2f00886 100644
@@ -582,8 +582,8 @@ static void print_conn_list(struct snd_info_buffer *buffer,
 
 	/* Get Cache connections info */
 	cache_len = snd_hda_get_conn_list(codec, nid, &list);
-	if (cache_len != conn_len
-			|| memcmp(list, conn, conn_len)) {
+	if (cache_len >= 0 && (cache_len != conn_len ||
+			      memcmp(list, conn, conn_len) != 0)) {
 		snd_iprintf(buffer, "  In-driver Connection: %d\n", cache_len);
 		if (cache_len > 0) {
 			snd_iprintf(buffer, "    ");
Applied
#4 lore raw

At Fri, 17 Apr 2015 16:19:46 +0300,

Quoted 9 lines · Dan Carpenter

This function is a bit unusual because it accepts negative values as "conn_len". It's theoretically possible for both "cache_len" and "conn_len" to be -ENOSPC and in that case we would oops trying to run memcmp() on the uninitialized "list" pointer.

---
v2: Don't print a negative error code as "In-driver Connection."

Applied, thanks.

Takashi

Quoted 17 lines
sound/pci/hda/hda_proc.c +2 -2
index ee62307..2f00886 100644
@@ -582,8 +582,8 @@ static void print_conn_list(struct snd_info_buffer *buffer,
 
 	/* Get Cache connections info */
 	cache_len = snd_hda_get_conn_list(codec, nid, &list);
-	if (cache_len != conn_len
-			|| memcmp(list, conn, conn_len)) {
+	if (cache_len >= 0 && (cache_len != conn_len ||
+			      memcmp(list, conn, conn_len) != 0)) {
 		snd_iprintf(buffer, "  In-driver Connection: %d\n", cache_len);
 		if (cache_len > 0) {
 			snd_iprintf(buffer, "    ");