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@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