6:15 a.m.
The virsysinfo changes caused Coverity to "take another look" at the code
and it found some issues. Each is described in the patch commit.
John Ferlan (2):
util: Resource some resource leaks in virsysinfo code
util: Avoid possible NULL dereference in virSysinfoParsePPCProcessor
src/util/virsysinfo.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
--
2.9.3
6:15 a.m.
New subject: [libvirt] [PATCH 1/2] util: Resource some resource leaks in virsysinfo code
Calls to virFileReadAll after a VIR_ALLOC that return NULL all show
a memory leak since 'ret' isn't virSysinfoDefFree'd and normal path
"return ret" doesn't free outbuf.
Reported by Coverity
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virsysinfo.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 7b54bda..14c17a8 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -270,7 +270,7 @@ virSysinfoReadPPC(void)
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
- return NULL;
+ goto no_memory;
}
ret->nprocessor = 0;
@@ -281,10 +281,12 @@ virSysinfoReadPPC(void)
if (virSysinfoParsePPCSystem(outbuf, &ret->system) < 0)
goto no_memory;
+ VIR_FREE(outbuf);
return ret;
no_memory:
VIR_FREE(outbuf);
+ virSysinfoDefFree(ret);
return NULL;
}
@@ -402,7 +404,7 @@ virSysinfoReadARM(void)
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
- return NULL;
+ goto no_memory;
}
ret->nprocessor = 0;
@@ -413,10 +415,12 @@ virSysinfoReadARM(void)
if (virSysinfoParseARMSystem(outbuf, &ret->system) < 0)
goto no_memory;
+ VIR_FREE(outbuf);
return ret;
no_memory:
VIR_FREE(outbuf);
+ virSysinfoDefFree(ret);
return NULL;
}
@@ -539,7 +543,7 @@ virSysinfoReadS390(void)
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, &outbuf) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
- return NULL;
+ goto no_memory;
}
ret->nprocessor = 0;
@@ -554,12 +558,13 @@ virSysinfoReadS390(void)
if (virFileReadAll(SYSINFO, 8192, &outbuf) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), SYSINFO);
- return NULL;
+ goto no_memory;
}
if (virSysinfoParseS390System(outbuf, &ret->system) < 0)
goto no_memory;
+ VIR_FREE(outbuf);
return ret;
no_memory:
--
2.9.3
6:15 a.m.
New subject: [libvirt] [PATCH 2/2] util: Avoid possible NULL dereference in virSysinfoParsePPCProcessor
Found by Coverity. Because there's an "if ((cur = strstr(base,
"revision"))
!= NULL) {" followed by a "base = cur" coverity notes that 'base'
could
then be NULL causing the return to the top of the "while ((tmp_base =
strstr(base, "processor")) != NULL) {" to have strstr deref a NULL
'base'
pointer because the setting of base at the bottom of the loop is unconditional.
Alter the code to set "base = cur" after processing each key. That will
"ensure" that base doesn't get set to NULL if both "cpu" and
"revision"
do no follow a "processor".
While a /proc/cpuinfo file that has a "processor" key but with neither
a "cpu" nor a "revision" doesn't seem feasible, the code is
written as if
it could happen, so we have to account for it.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/util/virsysinfo.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 14c17a8..8d3377c 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -231,6 +231,7 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDefPtr ret)
if (eol && VIR_STRNDUP(processor->processor_socket_destination,
cur, eol - cur) < 0)
return -1;
+ base = cur;
if ((cur = strstr(base, "cpu")) != NULL) {
cur = strchr(cur, ':') + 1;
@@ -239,6 +240,7 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDefPtr ret)
if (eol && VIR_STRNDUP(processor->processor_type,
cur, eol - cur) < 0)
return -1;
+ base = cur;
}
if ((cur = strstr(base, "revision")) != NULL) {
@@ -248,9 +250,9 @@ virSysinfoParsePPCProcessor(const char *base, virSysinfoDefPtr ret)
if (eol && VIR_STRNDUP(processor->processor_version,
cur, eol - cur) < 0)
return -1;
+ base = cur;
}
- base = cur;
}
return 0;
--
2.9.3
2848
days inactive
2848
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa