On 08/16/13 09:47, Alex Jia wrote: > This issue is introduced by commit 0fc8909, the virBitmapIsSet() needs caller > to ensure 'b < bitmap->max_bit', but it's lost in the virBitmapParse() caller, > this will cause crash of libvirtd, with the patch, libvirtd no crash and can > get a expected error "Failed to parse nodeset". > > How to reproduce? > > # virsh numatune foo --nodeset 1000000000 > Actual result: > error: Unable to change numa parameters > error: End of file while reading data: Input/output error > error: One or more references were leaked after disconnect from the hypervisor > error: Failed to reconnect to the hypervisor > > GDB backtrace: > .... > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=997367 > > Signed-off-by: Alex Jia <ajia@xxxxxxxxxx> > --- > The caller virBitmapGetBit() can make sure 'b < bitmap->max_bit', so don't > need to worry about higher caller for the virBitmapGetBit(), but the > virBitmapParse() is called by many XML parser function, not sure which one > can crash libvirtd with read-only client then probably require a CVE, I haven't > a good way to check them now and only manually check them one by one. > > src/util/virbitmap.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c > index 7e1cd02..edbfb30 100644 > --- a/src/util/virbitmap.c > +++ b/src/util/virbitmap.c > @@ -337,6 +337,9 @@ virBitmapParse(const char *str, > if (start < 0) > goto parse_error; > > + if ((*bitmap)->max_bit <= start) > + goto parse_error; > + This will check only the start value of a range. When you use a range that starts with a valid number "1-1000000000000" for example, then the loop that is setting bits from the range will triger the crash too. IMO the correct fix here is to get rid of the weird construction: if (!virBitmapIsSet(*bitmap, i)) { ignore_value(virBitmapSetBit(*bitmap, i)); ret++; } with if (virBitmapSetBit(*bitmap, i) < 0) goto error; and at the end count the enabled bits as ret = virBitmapCountBits(*bitmap); instead of the code that is present. I'll post an updated version of this patch containing the suggested changes. Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list