2:47 a.m.
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:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f80ee533700 (LWP 4156)]
0x00007f80fb1b8906 in virBitmapIsSet (str=<value optimized out>, terminator=0
'\000', bitmap=0x7f80ee532978, bitmapSize=<value optimized out>)
at util/virbitmap.c:158
158 return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b));
(gdb) bt
#0 0x00007f80fb1b8906 in virBitmapIsSet (str=<value optimized out>, terminator=0
'\000', bitmap=0x7f80ee532978, bitmapSize=<value optimized out>)
at util/virbitmap.c:158
#1 virBitmapParse (str=<value optimized out>, terminator=0 '\000',
bitmap=0x7f80ee532978, bitmapSize=<value optimized out>) at util/virbitmap.c:351
#2 0x00007f80e5e7ef6b in qemuDomainSetNumaParameters (dom=<value optimized out>,
params=<value optimized out>, nparams=1, flags=2) at qemu/qemu_driver.c:8357
#3 0x00007f80fb2ad7c6 in virDomainSetNumaParameters (domain=0x7f80d4000930,
params=0x7f80d4000b00, nparams=1, flags=0) at libvirt.c:4163
#4 0x00007f80fbcd17ec in remoteDispatchDomainSetNumaParameters (server=<value
optimized out>, client=<value optimized out>, msg=<value optimized out>,
rerr=0x7f80ee532ba0, args=<value optimized out>, ret=<value optimized
out>) at remote_dispatch.h:7771
#5 remoteDispatchDomainSetNumaParametersHelper (server=<value optimized out>,
client=<value optimized out>, msg=<value optimized out>, rerr=0x7f80ee532ba0,
args=<value optimized out>, ret=<value optimized out>) at
remote_dispatch.h:7741
#6 0x00007f80fb2f6518 in virNetServerProgramDispatchCall (prog=0x7f80fd33a990,
server=0x7f80fd32f3e0, client=0x7f80fd33fe30, msg=0x7f80fd33e550)
at rpc/virnetserverprogram.c:435
#7 virNetServerProgramDispatch (prog=0x7f80fd33a990, server=0x7f80fd32f3e0,
client=0x7f80fd33fe30, msg=0x7f80fd33e550) at rpc/virnetserverprogram.c:305
#8 0x00007f80fb2f9dd6 in virNetServerProcessMsg (srv=<value optimized out>,
client=0x7f80fd33fe30, prog=<value optimized out>, msg=0x7f80fd33e550)
at rpc/virnetserver.c:165
#9 0x00007f80fb2fa9f3 in virNetServerHandleJob (jobOpaque=<value optimized out>,
opaque=<value optimized out>) at rpc/virnetserver.c:186
#10 0x00007f80fb20210e in virThreadPoolWorker (opaque=<value optimized out>) at
util/virthreadpool.c:144
#11 0x00007f80fb2016f6 in virThreadHelper (data=<value optimized out>) at
util/virthreadpthread.c:161
#12 0x00007f80f8d857f1 in start_thread (arg=0x7f80ee533700) at pthread_create.c:301
#13 0x00007f80f86bb70d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=997367
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
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;
+
cur = tmp;
virSkipSpaces(&cur);
--
1.7.1
4:16 a.m.
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(a)redhat.com>
---
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
6:42 a.m.
On 08/16/2013 01:47 AM, 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".
---
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.
If you are worried that a bug might be a CVE, it is best to practice
responsible disclosure, and NOT post the patch upstream, but instead
post to libvirt-security(a)redhat.com. That way, the problem can be
discussed without public disclosure, rather than calling attention to
the fact and making it easier to design a 0-day exploit. But now that
this is already publicly disclosed, we have to hurry up both the fix,
and our analysis of whether it is exploitable.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:21 p.m.
On 08/16/2013 07:42 PM, Eric Blake wrote:
On 08/16/2013 01:47 AM, 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".
>
> ---
> 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.
If you are worried that a bug might be a CVE, it is best to practice
responsible disclosure, and NOT post the patch upstream, but instead
post to libvirt-security(a)redhat.com. That way, the problem can be
Got it, if I think a bug might be a CVE then will post the patch to
libvirt-security(a)redhat.com next time, thanks.
discussed without public disclosure, rather than calling attention
to
the fact and making it easier to design a 0-day exploit. But now that
this is already publicly disclosed, we have to hurry up both the fix,
and our analysis of whether it is exploitable.
4113
days inactive
4116
days old
3 comments
3 participants
participants (3)
-
Alex Jia -
Eric Blake -
Peter Krempa