Re: [libvirt] [PATCH 1/6] Avoid Coverity FORWARD_NULL prior to strtok_r calls
On Wed, Sep 23, 2015 at 07:18:28PM -0400, John Ferlan wrote:
The 'strtok_r' function requires passing a NULL as the first
parameter
on subsequent calls in order to ensure the code picks up where it left
off on a previous call. However, Coverity doesn't quite realize this
and points out that if a NULL was passed in as the third argument it would
result in a possible NULL deref because the strtok_r function will assign
the third argument to the first in the call is NULL.
The description seems contradictory to the patch. From the asserts that
fix it, I'd assume Coverity has a problem with the first argument
possibly being NULL - usually obtained from a library function Coverity
doesn't understand.
Adding an sa_assert() prior to each initial call quiets Coverity
Thus rendering the coverity scan useless.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/esx/esx_vi.c | 1 +
src/libxl/libxl_conf.c | 1 +
src/openvz/openvz_conf.c | 2 ++
src/xenapi/xenapi_utils.c | 1 +
4 files changed, 5 insertions(+)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index af822b1..a57d753 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -1173,6 +1173,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const
char *path)
goto cleanup;
/* Lookup Datacenter */
+ sa_assert(tmp);
This should be asserted in the caller. It seems priv->parsedUri->path
can be NULL in esxConnectToVCenter, but this function is not called in
that case.
item = strtok_r(tmp, "/", &saveptr);
if (!item) {
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 35fd495..ab588e8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -353,6 +353,7 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
/* Split capabilities string into tokens. strtok_r is OK here because
* we "own" the buffer. Parse out the features from each token.
*/
+ sa_assert(ver_info->capabilities);
This one should be closer to the function that filled out the string.
for (str = ver_info->capabilities, nr_guest_archs = 0;
nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0])
&& (token = strtok_r(str, " ", &saveptr)) !=
NULL;
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index db0a9a7..003272e 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -138,6 +138,7 @@ openvzParseBarrierLimit(const char* value,
char *str;
int ret = -1;
+ sa_assert(value);
Same here.
if (VIR_STRDUP(str, value) < 0)
goto error;
@@ -965,6 +966,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len)
}
}
+ sa_assert(line);
This one already is right after getline.
iden = strtok_r(line, " ", &saveptr);
uuidbuf = strtok_r(NULL, "\n", &saveptr);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80e084..6b710cd 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -301,6 +301,7 @@ getCpuBitMapfromString(char *mask, unsigned char *cpumap, int
maplen)
int max_bits = maplen * 8;
char *num = NULL, *bp = NULL;
bzero(cpumap, maplen);
+ sa_assert(mask);
num = strtok_r(mask, ",", &bp);
while (num != NULL) {
if (virStrToLong_i(num, NULL, 10, &pos) < 0)
virBitmap* functions can be used instead.
Jan
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)