External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-08 05:55:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

virbitmap: Refactor virBitmapParse to avoid access beyond bounds of array

Browse Source 
The virBitmapParse function was calling virBitmapIsSet() function that
requires the caller to check the bounds of the bitmap without checking
them. This resulted into crashes when parsing a bitmap string that was
exceeding the bounds used as argument.

This patch refactors the function to use virBitmapSetBit without
checking if the bit is set (this function does the checks internally)
and then counts the bits in the bitmap afterwards (instead of keeping
track while parsing the string).

This patch also changes the "parse_error" label to a more common
"error".

The refactor should also get rid of the need to call sa_assert on the
returned variable as the callpath should allow coverity to infer the
possible return values.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997367

Thanks to Alex Jia for tracking down the issue. This issue is introduced
by commit 0fc8909.

(cherry picked from commit 47b9127e88)
This commit is contained in:
Peter Krempa 2013-08-16 12:22:32 +02:00
parent ed5274ca2e
commit 1ffdaced5b
1 changed files with 15 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
src/util/virbitmap.c
View File

@ -290,7 +290,6 @@ virBitmapParse(const char *str,
virBitmapPtr *bitmap,
size_t bitmapSize)
{
int ret = 0;
bool neg = false;
const char *cur;
char *tmp;
@ -322,12 +321,12 @@ virBitmapParse(const char *str,
}
if (!c_isdigit(*cur))
goto parse_error;
goto error;
if (virStrToLong_i(cur, &tmp, 10, &start) < 0)
goto parse_error;
goto error;
if (start < 0)
goto parse_error;
goto error;
cur = tmp;
@ -335,35 +334,29 @@ virBitmapParse(const char *str,
if (*cur == ',' || *cur == 0 || *cur == terminator) {
if (neg) {
if (virBitmapIsSet(*bitmap, start)) {
ignore_value(virBitmapClearBit(*bitmap, start));
ret--;
}
if (virBitmapClearBit(*bitmap, start) < 0)
goto error;
} else {
if (!virBitmapIsSet(*bitmap, start)) {
ignore_value(virBitmapSetBit(*bitmap, start));
ret++;
}
if (virBitmapSetBit(*bitmap, start) < 0)
goto error;
}
} else if (*cur == '-') {
if (neg)
goto parse_error;
goto error;
cur++;
virSkipSpaces(&cur);
if (virStrToLong_i(cur, &tmp, 10, &last) < 0)
goto parse_error;
goto error;
if (last < start)
goto parse_error;
goto error;
cur = tmp;
for (i = start; i <= last; i++) {
if (!virBitmapIsSet(*bitmap, i)) {
ignore_value(virBitmapSetBit(*bitmap, i));
ret++;
}
if (virBitmapSetBit(*bitmap, i) < 0)
goto error;
}
virSkipSpaces(&cur);
@ -376,14 +369,13 @@ virBitmapParse(const char *str,
} else if (*cur == 0 || *cur == terminator) {
break;
} else {
goto parse_error;
goto error;
}
}
sa_assert(ret >= 0);
return ret;
return virBitmapCountBits(*bitmap);
parse_error:
error:
virBitmapFree(*bitmap);
*bitmap = NULL;
return -1;