| Message ID | 1260824004-2941-6-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
Luiz Capitulino <lcapitulino@redhat.com> writes: > The monitor_vprintf() function now touches the 'mon' pointer > before calling monitor_puts(), this causes block migration > to segfault as its functions call monitor_printf() with a > NULL 'mon'. I figure this worked fine until commit 4a29a85d made monitor_vprintf() dereference mon. > To fix the problem this commit moves the 'mon' NULL check > from monitor_puts() to monitor_vprintf(). > > This can potentially hide bugs, but for some reason this has > been the behavior for a long time. > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > monitor.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/monitor.c b/monitor.c > index b518cc4..ebd0282 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -177,9 +177,6 @@ static void monitor_puts(Monitor *mon, const char *str) > { > char c; > > - if (!mon) > - return; > - > for(;;) { > c = *str++; > if (c == '\0') > @@ -195,6 +192,9 @@ static void monitor_puts(Monitor *mon, const char *str) > > void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) > { > + if (!mon) > + return; > + > if (mon->mc && !mon->mc->print_enabled) { > qemu_error_new(QERR_UNDEFINED_ERROR); > } else { There are no other callers of monitor_puts(), so removing the check there is okay. Before the code motion, we throw QERR_UNDEFINED_ERROR on monitor_vprintf(NULL, ...). Afterwards, we don't. Could you explain why that's okay?
On Tue, 15 Dec 2009 10:42:46 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Before the code motion, we throw QERR_UNDEFINED_ERROR on > monitor_vprintf(NULL, ...). Afterwards, we don't. Could you explain > why that's okay? We never did that. A call like that will just segfault QEMU w/o this fix.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Tue, 15 Dec 2009 10:42:46 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Before the code motion, we throw QERR_UNDEFINED_ERROR on >> monitor_vprintf(NULL, ...). Afterwards, we don't. Could you explain >> why that's okay? > > We never did that. A call like that will just segfault QEMU > w/o this fix. You're right. Brain fart on my part.
diff --git a/monitor.c b/monitor.c index b518cc4..ebd0282 100644 --- a/monitor.c +++ b/monitor.c @@ -177,9 +177,6 @@ static void monitor_puts(Monitor *mon, const char *str) { char c; - if (!mon) - return; - for(;;) { c = *str++; if (c == '\0') @@ -195,6 +192,9 @@ static void monitor_puts(Monitor *mon, const char *str) void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { + if (!mon) + return; + if (mon->mc && !mon->mc->print_enabled) { qemu_error_new(QERR_UNDEFINED_ERROR); } else {
The monitor_vprintf() function now touches the 'mon' pointer before calling monitor_puts(), this causes block migration to segfault as its functions call monitor_printf() with a NULL 'mon'. To fix the problem this commit moves the 'mon' NULL check from monitor_puts() to monitor_vprintf(). This can potentially hide bugs, but for some reason this has been the behavior for a long time. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- monitor.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)