[alsa-devel] [patch] 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.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index ce5a6da..387fdfc 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -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, Dan Carpenter wrote:
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
My static checker complains about this.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index ce5a6da..387fdfc 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -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 ||
snd_iprintf(buffer, " In-driver Connection: %d\n", cache_len); if (cache_len > 0) { snd_iprintf(buffer, " ");memcmp(list, conn, conn_len) != 0) {
On Fri, Apr 17, 2015 at 02:42:05PM +0200, Takashi Iwai wrote:
At Fri, 17 Apr 2015 15:35:15 +0300, Dan Carpenter wrote:
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?
Sure. Will do.
regards, 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.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- v2: Don't print a negative error code as "In-driver Connection."
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index ee62307..2f00886 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -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, " ");
At Fri, 17 Apr 2015 16:19:46 +0300, Dan Carpenter wrote:
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.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
v2: Don't print a negative error code as "In-driver Connection."
Applied, thanks.
Takashi
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index ee62307..2f00886 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -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 ||
snd_iprintf(buffer, " In-driver Connection: %d\n", cache_len); if (cache_len > 0) { snd_iprintf(buffer, " ");memcmp(list, conn, conn_len) != 0)) {
participants (2)
-
Dan Carpenter
-
Takashi Iwai