* [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine
@ 2017-02-17 8:27 Ziyue Yang
2017-02-17 8:27 ` [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU Ziyue Yang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Ziyue Yang @ 2017-02-17 8:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Dr . David Alan Gilbert, Pavel Butsykin, Ziyue Yang
From: Ziyue Yang <yzylivezh@hotmail.com>
Many QEMU monitor commands, like "info lapic", "info tlb" and so on
use mon_get_cpu or related wrappers to access CPU info without checking
whether the CPU exists.
This patch series fix the "info lapic" case, and is the base of the incoming
patch series aiming to eliminate segfaults caused by other QEMU commands
trying to access CPU that doesn't exist.
Ziyue Yang (2):
monitor.c: make mon_get_cpu return NULL when there is no CPU
target/i386/monitor.c: check return value of mon_get_cpu before using
it
monitor.c | 10 +++++++---
target/i386/monitor.c | 7 +++++--
2 files changed, 12 insertions(+), 5 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU 2017-02-17 8:27 [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Ziyue Yang @ 2017-02-17 8:27 ` Ziyue Yang 2017-02-19 3:55 ` Philippe Mathieu-Daudé 2017-02-17 8:27 ` [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it Ziyue Yang 2017-02-20 6:07 ` [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Thomas Huth 2 siblings, 1 reply; 9+ messages in thread From: Ziyue Yang @ 2017-02-17 8:27 UTC (permalink / raw) To: qemu-devel Cc: Dr . David Alan Gilbert, Pavel Butsykin, Ziyue Yang, Ziyue Yang From: Ziyue Yang <yzylivezh@hotmail.com> Currently mon_get_cpu always dereferences first_cpu without checking whether it's a valid pointer. This commit adds check before dereferencing, and reports "No CPU" info if there isn't any CPU then returns NULL. Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> --- monitor.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/monitor.c b/monitor.c index 3cd72a9bab..6b25cf7a2b 100644 --- a/monitor.c +++ b/monitor.c @@ -1026,6 +1026,10 @@ int monitor_set_cpu(int cpu_index) CPUState *mon_get_cpu(void) { if (!cur_mon->mon_cpu) { + if (!first_cpu) { + monitor_printf(cur_mon, "No CPU available on this machine\n"); + return NULL; + } monitor_set_cpu(first_cpu->cpu_index); } cpu_synchronize_state(cur_mon->mon_cpu); @@ -2495,11 +2499,11 @@ static int default_fmt_size = 4; static int is_valid_option(const char *c, const char *typestr) { char option[3]; - + option[0] = '-'; option[1] = *c; option[2] = '\0'; - + typestr = strstr(typestr, option); return (typestr != NULL); } @@ -2864,7 +2868,7 @@ static QDict *monitor_parse_arguments(Monitor *mon, p++; if(c != *p) { if(!is_valid_option(p, typestr)) { - + monitor_printf(mon, "%s: unsupported option -%c\n", cmd->name, *p); goto fail; -- 2.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU 2017-02-17 8:27 ` [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU Ziyue Yang @ 2017-02-19 3:55 ` Philippe Mathieu-Daudé 2017-02-20 6:09 ` Thomas Huth 0 siblings, 1 reply; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2017-02-19 3:55 UTC (permalink / raw) To: Ziyue Yang Cc: qemu-devel, Ziyue Yang, Dr . David Alan Gilbert, Pavel Butsykin On 02/17/2017 05:27 AM, Ziyue Yang wrote: > From: Ziyue Yang <yzylivezh@hotmail.com> > > Currently mon_get_cpu always dereferences first_cpu without checking > whether it's a valid pointer. This commit adds check before dereferencing, > and reports "No CPU" info if there isn't any CPU then returns NULL. > > Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > monitor.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 3cd72a9bab..6b25cf7a2b 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1026,6 +1026,10 @@ int monitor_set_cpu(int cpu_index) > CPUState *mon_get_cpu(void) > { > if (!cur_mon->mon_cpu) { > + if (!first_cpu) { > + monitor_printf(cur_mon, "No CPU available on this machine\n"); > + return NULL; > + } > monitor_set_cpu(first_cpu->cpu_index); > } > cpu_synchronize_state(cur_mon->mon_cpu); > @@ -2495,11 +2499,11 @@ static int default_fmt_size = 4; > static int is_valid_option(const char *c, const char *typestr) > { > char option[3]; > - > + > option[0] = '-'; > option[1] = *c; > option[2] = '\0'; > - > + > typestr = strstr(typestr, option); > return (typestr != NULL); > } > @@ -2864,7 +2868,7 @@ static QDict *monitor_parse_arguments(Monitor *mon, > p++; > if(c != *p) { > if(!is_valid_option(p, typestr)) { > - > + > monitor_printf(mon, "%s: unsupported option -%c\n", > cmd->name, *p); > goto fail; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU 2017-02-19 3:55 ` Philippe Mathieu-Daudé @ 2017-02-20 6:09 ` Thomas Huth 2017-02-20 9:33 ` Yang Ziyue 0 siblings, 1 reply; 9+ messages in thread From: Thomas Huth @ 2017-02-20 6:09 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Ziyue Yang Cc: Ziyue Yang, Pavel Butsykin, qemu-devel, Dr . David Alan Gilbert On 19.02.2017 04:55, Philippe Mathieu-Daudé wrote: > On 02/17/2017 05:27 AM, Ziyue Yang wrote: >> From: Ziyue Yang <yzylivezh@hotmail.com> >> >> Currently mon_get_cpu always dereferences first_cpu without checking >> whether it's a valid pointer. This commit adds check before >> dereferencing, >> and reports "No CPU" info if there isn't any CPU then returns NULL. >> >> Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> --- >> monitor.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/monitor.c b/monitor.c >> index 3cd72a9bab..6b25cf7a2b 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -1026,6 +1026,10 @@ int monitor_set_cpu(int cpu_index) >> CPUState *mon_get_cpu(void) >> { >> if (!cur_mon->mon_cpu) { >> + if (!first_cpu) { >> + monitor_printf(cur_mon, "No CPU available on this >> machine\n"); >> + return NULL; >> + } >> monitor_set_cpu(first_cpu->cpu_index); >> } >> cpu_synchronize_state(cur_mon->mon_cpu); >> @@ -2495,11 +2499,11 @@ static int default_fmt_size = 4; >> static int is_valid_option(const char *c, const char *typestr) >> { >> char option[3]; >> - >> + >> option[0] = '-'; >> option[1] = *c; >> option[2] = '\0'; >> - >> + >> typestr = strstr(typestr, option); >> return (typestr != NULL); >> } >> @@ -2864,7 +2868,7 @@ static QDict *monitor_parse_arguments(Monitor *mon, >> p++; >> if(c != *p) { >> if(!is_valid_option(p, typestr)) { >> - >> + >> monitor_printf(mon, "%s: unsupported >> option -%c\n", >> cmd->name, *p); >> goto fail; Your patch contains some unnecessary white space changes, please try to avoid that! (or send a separate "beautification" patch to fix these). Thomas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU 2017-02-20 6:09 ` Thomas Huth @ 2017-02-20 9:33 ` Yang Ziyue 0 siblings, 0 replies; 9+ messages in thread From: Yang Ziyue @ 2017-02-20 9:33 UTC (permalink / raw) To: Thomas Huth Cc: Philippe Mathieu-Daudé, Ziyue Yang, Pavel Butsykin, qemu-devel, Dr . David Alan Gilbert Sorry for forgetting the "no irrelevant change" rule...won't do that next time! 2017-02-20 14:09 GMT+08:00 Thomas Huth <thuth@redhat.com>: > On 19.02.2017 04:55, Philippe Mathieu-Daudé wrote: >> On 02/17/2017 05:27 AM, Ziyue Yang wrote: >>> From: Ziyue Yang <yzylivezh@hotmail.com> >>> >>> Currently mon_get_cpu always dereferences first_cpu without checking >>> whether it's a valid pointer. This commit adds check before >>> dereferencing, >>> and reports "No CPU" info if there isn't any CPU then returns NULL. >>> >>> Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> >> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >>> --- >>> monitor.c | 10 +++++++--- >>> 1 file changed, 7 insertions(+), 3 deletions(-) >>> >>> diff --git a/monitor.c b/monitor.c >>> index 3cd72a9bab..6b25cf7a2b 100644 >>> --- a/monitor.c >>> +++ b/monitor.c >>> @@ -1026,6 +1026,10 @@ int monitor_set_cpu(int cpu_index) >>> CPUState *mon_get_cpu(void) >>> { >>> if (!cur_mon->mon_cpu) { >>> + if (!first_cpu) { >>> + monitor_printf(cur_mon, "No CPU available on this >>> machine\n"); >>> + return NULL; >>> + } >>> monitor_set_cpu(first_cpu->cpu_index); >>> } >>> cpu_synchronize_state(cur_mon->mon_cpu); >>> @@ -2495,11 +2499,11 @@ static int default_fmt_size = 4; >>> static int is_valid_option(const char *c, const char *typestr) >>> { >>> char option[3]; >>> - >>> + >>> option[0] = '-'; >>> option[1] = *c; >>> option[2] = '\0'; >>> - >>> + >>> typestr = strstr(typestr, option); >>> return (typestr != NULL); >>> } >>> @@ -2864,7 +2868,7 @@ static QDict *monitor_parse_arguments(Monitor *mon, >>> p++; >>> if(c != *p) { >>> if(!is_valid_option(p, typestr)) { >>> - >>> + >>> monitor_printf(mon, "%s: unsupported >>> option -%c\n", >>> cmd->name, *p); >>> goto fail; > > Your patch contains some unnecessary white space changes, please try to > avoid that! (or send a separate "beautification" patch to fix these). > > Thomas > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it 2017-02-17 8:27 [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Ziyue Yang 2017-02-17 8:27 ` [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU Ziyue Yang @ 2017-02-17 8:27 ` Ziyue Yang 2017-02-19 3:56 ` Philippe Mathieu-Daudé 2017-02-20 6:07 ` [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Thomas Huth 2 siblings, 1 reply; 9+ messages in thread From: Ziyue Yang @ 2017-02-17 8:27 UTC (permalink / raw) To: qemu-devel Cc: Dr . David Alan Gilbert, Pavel Butsykin, Ziyue Yang, Ziyue Yang From: Ziyue Yang <yzylivezh@hotmail.com> This patch eliminates the segfault caused by accessing CPU that doesn't exist in hmp command "info lapic", which can be reproduced by $ qemu-system-x86_64 -nographic -M none -serial none -monitor stdio and then type "info lapic" into qemu monitor. Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> --- target/i386/monitor.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/target/i386/monitor.c b/target/i386/monitor.c index 468aa073bc..7b96c74a24 100644 --- a/target/i386/monitor.c +++ b/target/i386/monitor.c @@ -624,8 +624,11 @@ const MonitorDef *target_monitor_defs(void) void hmp_info_local_apic(Monitor *mon, const QDict *qdict) { - x86_cpu_dump_local_apic_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, - CPU_DUMP_FPU); + CPUState *cs = mon_get_cpu(); + if (cs) { + x86_cpu_dump_local_apic_state(cs, (FILE *)mon, monitor_fprintf, + CPU_DUMP_FPU); + } } void hmp_info_io_apic(Monitor *mon, const QDict *qdict) -- 2.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it 2017-02-17 8:27 ` [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it Ziyue Yang @ 2017-02-19 3:56 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 9+ messages in thread From: Philippe Mathieu-Daudé @ 2017-02-19 3:56 UTC (permalink / raw) To: Ziyue Yang, qemu-devel Cc: Ziyue Yang, Dr . David Alan Gilbert, Pavel Butsykin On 02/17/2017 05:27 AM, Ziyue Yang wrote: > From: Ziyue Yang <yzylivezh@hotmail.com> > > This patch eliminates the segfault caused by accessing CPU that doesn't > exist in hmp command "info lapic", which can be reproduced by > > $ qemu-system-x86_64 -nographic -M none -serial none -monitor stdio > > and then type "info lapic" into qemu monitor. > > Signed-off-by: Ziyue Yang <skiver.cloud.yzy@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/i386/monitor.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/target/i386/monitor.c b/target/i386/monitor.c > index 468aa073bc..7b96c74a24 100644 > --- a/target/i386/monitor.c > +++ b/target/i386/monitor.c > @@ -624,8 +624,11 @@ const MonitorDef *target_monitor_defs(void) > > void hmp_info_local_apic(Monitor *mon, const QDict *qdict) > { > - x86_cpu_dump_local_apic_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, > - CPU_DUMP_FPU); > + CPUState *cs = mon_get_cpu(); > + if (cs) { > + x86_cpu_dump_local_apic_state(cs, (FILE *)mon, monitor_fprintf, > + CPU_DUMP_FPU); > + } > } > > void hmp_info_io_apic(Monitor *mon, const QDict *qdict) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine 2017-02-17 8:27 [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Ziyue Yang 2017-02-17 8:27 ` [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU Ziyue Yang 2017-02-17 8:27 ` [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it Ziyue Yang @ 2017-02-20 6:07 ` Thomas Huth 2017-02-21 19:42 ` Dr. David Alan Gilbert 2 siblings, 1 reply; 9+ messages in thread From: Thomas Huth @ 2017-02-20 6:07 UTC (permalink / raw) To: Ziyue Yang, qemu-devel Cc: Ziyue Yang, Dr . David Alan Gilbert, Pavel Butsykin On 17.02.2017 09:27, Ziyue Yang wrote: > From: Ziyue Yang <yzylivezh@hotmail.com> > > Many QEMU monitor commands, like "info lapic", "info tlb" and so on > use mon_get_cpu or related wrappers to access CPU info without checking > whether the CPU exists. > This patch series fix the "info lapic" case, and is the base of the incoming > patch series aiming to eliminate segfaults caused by other QEMU commands > trying to access CPU that doesn't exist. Hi, FYI, I've posted a patch for all of these monitor commands that crash without CPU already last month: https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg02602.html Thomas ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine 2017-02-20 6:07 ` [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Thomas Huth @ 2017-02-21 19:42 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 9+ messages in thread From: Dr. David Alan Gilbert @ 2017-02-21 19:42 UTC (permalink / raw) To: Thomas Huth; +Cc: Ziyue Yang, qemu-devel, Ziyue Yang, Pavel Butsykin * Thomas Huth (thuth@redhat.com) wrote: > On 17.02.2017 09:27, Ziyue Yang wrote: > > From: Ziyue Yang <yzylivezh@hotmail.com> > > > > Many QEMU monitor commands, like "info lapic", "info tlb" and so on > > use mon_get_cpu or related wrappers to access CPU info without checking > > whether the CPU exists. > > This patch series fix the "info lapic" case, and is the base of the incoming > > patch series aiming to eliminate segfaults caused by other QEMU commands > > trying to access CPU that doesn't exist. > > Hi, > > FYI, I've posted a patch for all of these monitor commands that crash > without CPU already last month: > > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg02602.html I've just sent that in my HMP pull: https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg04939.html Ziyue Yang: Perhaps it's best to compare with the things in that pull and check to see if you have any items that were not in that pull. Dave > Thomas > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-21 19:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-17 8:27 [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Ziyue Yang 2017-02-17 8:27 ` [Qemu-devel] [PATCH 1/2] monitor.c: make mon_get_cpu return NULL when there is no CPU Ziyue Yang 2017-02-19 3:55 ` Philippe Mathieu-Daudé 2017-02-20 6:09 ` Thomas Huth 2017-02-20 9:33 ` Yang Ziyue 2017-02-17 8:27 ` [Qemu-devel] [PATCH 2/2] target/i386/monitor.c: check return value of mon_get_cpu before using it Ziyue Yang 2017-02-19 3:56 ` Philippe Mathieu-Daudé 2017-02-20 6:07 ` [Qemu-devel] [PATCH 0/2] fix segfaults caused by accessing CPU in empty machine Thomas Huth 2017-02-21 19:42 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).