ALSA: hda - potential (but unlikely) uninitialized variable
Subsystems (2)
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
@@ -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," " );
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-ENOSPCand in that case we would oops trying to runmemcmp()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.
Signed-off-by Dan Carpenter <[email protected]>sound/pci/hda/hda_proc.c +2
-2@@ -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," " );
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-ENOSPCand in that case we would oops trying to runmemcmp()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
PATCH v2 1/1 ALSA: hda - potential (but unlikely) uninitialized variable
In reply to Takashi IwaiThis 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
@@ -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," " );
Takashi Iwai
AppliedAt 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-ENOSPCand in that case we would oops trying to runmemcmp()on the uninitialized "list" pointer.Signed-off-by Dan Carpenter <[email protected]>---
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@@ -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," " );