On Wed, Oct 12, 2011 at 06:21:37PM -0600, Eric Blake wrote:

Detected by Coverity. If, for some reason, our text monitor input does not match our assumptions, we end up incrementing p while it is NULL, then dereferencing the pointer 0x1, which will fault.

* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBlockStatsParamsNumber): Rewrite to avoid deref of strchr failure. Fix indentation. --- src/qemu/qemu_monitor_text.c | 33 +++++++++++++++------------------ 1 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 51e8c5c..1eb9846 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1036,26 +1036,23 @@ int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon, * "floppy0: ") */ p = strchr(p, ' '); - p++;

okay, clearly that's dangerous

- while (*p) { - if (STRPREFIX (p, "rd_bytes=") || - STRPREFIX (p, "wr_bytes=") || - STRPREFIX (p, "rd_operations=") || - STRPREFIX (p, "wr_operations=") || - STRPREFIX (p, "rd_total_times_ns=") || - STRPREFIX (p, "wr_total_times_ns=") || - STRPREFIX (p, "flush_operations=") || - STRPREFIX (p, "flush_total_times_ns=")) { - num++; - } else { - VIR_DEBUG ("unknown block stat near %s", p); - } + while (p && p < eol) { + if (STRPREFIX (p, " rd_bytes=") || + STRPREFIX (p, " wr_bytes=") || + STRPREFIX (p, " rd_operations=") || + STRPREFIX (p, " wr_operations=") || + STRPREFIX (p, " rd_total_times_ns=") || + STRPREFIX (p, " wr_total_times_ns=") || + STRPREFIX (p, " flush_operations=") || + STRPREFIX (p, " flush_total_times_ns=")) { + num++; + } else { + VIR_DEBUG ("unknown block stat near %s", p); + }

- /* Skip to next label. */ - p = strchr (p, ' '); - if (!p || p >= eol) break; - p++; + /* Skip to next label. */ + p = strchr(p + 1, ' '); }

*nparams = num;

ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/