We read from QEMU until seeing a \r\n pair to indicate a completed reply
or event. To avoid memory denial-of-service though, we must have a size
limit on amount of data we buffer. 10 MB is large enough that it ought
to cope with normal QEMU replies, and small enough that we're not
consuming unreasonable mem.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
src/qemu/qemu_monitor.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 046caf001c..85c7d68a13 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -55,6 +55,15 @@ VIR_LOG_INIT("qemu.qemu_monitor");
#define DEBUG_IO 0
#define DEBUG_RAW_IO 0
+/* We read from QEMU until seeing a \r\n pair to indicate a
+ * completed reply or event. To avoid memory denial-of-service
+ * though, we must have a size limit on amount of data we
+ * buffer. 10 MB is large enough that it ought to cope with
+ * normal QEMU replies, and small enough that we're not
+ * consuming unreasonable mem.
+ */
+#define QEMU_MONITOR_MAX_RESPONSE (10 * 1024 * 1024)
+
struct _qemuMonitor {
virObjectLockable parent;
@@ -575,6 +584,12 @@ qemuMonitorIORead(qemuMonitorPtr mon)
int ret = 0;
if (avail < 1024) {
+ if (mon->bufferLength >= QEMU_MONITOR_MAX_RESPONSE) {
+ virReportSystemError(ERANGE,
+ _("No complete monitor response found in %d bytes"),
+ QEMU_MONITOR_MAX_RESPONSE);
+ return -1;
+ }
if (VIR_REALLOC_N(mon->buffer,
mon->bufferLength + 1024) < 0)
return -1;
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/16/2018 06:01 PM, Daniel P. Berrange wrote: > We read from QEMU until seeing a \r\n pair to indicate a completed reply > or event. To avoid memory denial-of-service though, we must have a size > limit on amount of data we buffer. 10 MB is large enough that it ought > to cope with normal QEMU replies, and small enough that we're not > consuming unreasonable mem. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > src/qemu/qemu_monitor.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 046caf001c..85c7d68a13 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -55,6 +55,15 @@ VIR_LOG_INIT("qemu.qemu_monitor"); > #define DEBUG_IO 0 > #define DEBUG_RAW_IO 0 > > +/* We read from QEMU until seeing a \r\n pair to indicate a > + * completed reply or event. To avoid memory denial-of-service > + * though, we must have a size limit on amount of data we > + * buffer. 10 MB is large enough that it ought to cope with > + * normal QEMU replies, and small enough that we're not > + * consuming unreasonable mem. > + */ > +#define QEMU_MONITOR_MAX_RESPONSE (10 * 1024 * 1024) > + > struct _qemuMonitor { > virObjectLockable parent; > > @@ -575,6 +584,12 @@ qemuMonitorIORead(qemuMonitorPtr mon) > int ret = 0; > > if (avail < 1024) { > + if (mon->bufferLength >= QEMU_MONITOR_MAX_RESPONSE) { > + virReportSystemError(ERANGE, > + _("No complete monitor response found in %d bytes"), > + QEMU_MONITOR_MAX_RESPONSE); > + return -1; > + } > if (VIR_REALLOC_N(mon->buffer, > mon->bufferLength + 1024) < 0) > return -1; > ACK, although is this really a CVE? Doesn't look that harmful to me. I mean, owning qemu is not that easy, is it? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jan 17, 2018 at 05:13:06PM +0100, Michal Privoznik wrote: > On 01/16/2018 06:01 PM, Daniel P. Berrange wrote: > > We read from QEMU until seeing a \r\n pair to indicate a completed reply > > or event. To avoid memory denial-of-service though, we must have a size > > limit on amount of data we buffer. 10 MB is large enough that it ought > > to cope with normal QEMU replies, and small enough that we're not > > consuming unreasonable mem. > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > src/qemu/qemu_monitor.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > > index 046caf001c..85c7d68a13 100644 > > --- a/src/qemu/qemu_monitor.c > > +++ b/src/qemu/qemu_monitor.c > > @@ -55,6 +55,15 @@ VIR_LOG_INIT("qemu.qemu_monitor"); > > #define DEBUG_IO 0 > > #define DEBUG_RAW_IO 0 > > > > +/* We read from QEMU until seeing a \r\n pair to indicate a > > + * completed reply or event. To avoid memory denial-of-service > > + * though, we must have a size limit on amount of data we > > + * buffer. 10 MB is large enough that it ought to cope with > > + * normal QEMU replies, and small enough that we're not > > + * consuming unreasonable mem. > > + */ > > +#define QEMU_MONITOR_MAX_RESPONSE (10 * 1024 * 1024) > > + > > struct _qemuMonitor { > > virObjectLockable parent; > > > > @@ -575,6 +584,12 @@ qemuMonitorIORead(qemuMonitorPtr mon) > > int ret = 0; > > > > if (avail < 1024) { > > + if (mon->bufferLength >= QEMU_MONITOR_MAX_RESPONSE) { > > + virReportSystemError(ERANGE, > > + _("No complete monitor response found in %d bytes"), > > + QEMU_MONITOR_MAX_RESPONSE); > > + return -1; > > + } > > if (VIR_REALLOC_N(mon->buffer, > > mon->bufferLength + 1024) < 0) > > return -1; > > > > ACK, although is this really a CVE? Doesn't look that harmful to me. I > mean, owning qemu is not that easy, is it? There are a never ending stream of CVEs in QEMU guest device models. These are mitigated by SELinux preventing a compromised QEMU from attacking resources on the host it isn't granted access to. So attention would naturally focus on attacking things it already has access to like the libvirt monitor connection. Memory denial of service in libvirt is not too serious, but still a CVE bug. Worse would be if libvirtd has buffer overflow/crash in parsing a JSON response... Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 01/17/2018 10:13 AM, Michal Privoznik wrote: > On 01/16/2018 06:01 PM, Daniel P. Berrange wrote: >> We read from QEMU until seeing a \r\n pair to indicate a completed reply >> or event. To avoid memory denial-of-service though, we must have a size >> limit on amount of data we buffer. 10 MB is large enough that it ought >> to cope with normal QEMU replies, and small enough that we're not >> consuming unreasonable mem. >> >> > > ACK, although is this really a CVE? Doesn't look that harmful to me. I > mean, owning qemu is not that easy, is it? We treat qemu as untrusted, in case a guest escapes qemu due to some other CVE. If a guest really did cause qemu to emit unbounded QMP text, and it starves libvirtd, then that guest has mounted a denial of service against anything else libvirtd is starved from doing. So yes, in my opinion it is a CVE, even if it is an unlikely case because it won't trigger without a flaw in more than one layer. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.