[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix crashes when using
From: |
Thomas Huth |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix crashes when using HMP commands without CPU |
Date: |
Thu, 12 Jan 2017 09:36:25 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 |
On 12.01.2017 09:10, Markus Armbruster wrote:
> Thomas Huth <address@hidden> writes:
>
>> When running certain HMP commands ("info registers", "info cpustats"
>> or dumping virtual memory) with the "none" machine, QEMU crashes
>> with a segmentation fault. This happens because the "none" machine does
>> not have any CPUs by default,
>
> "Sachen gibt's!"
>
>> but these HMP commands did not check for
>> a valid CPU pointer yet. Add such a check now and print a message
>> about the missing CPU instead.
>
> Have you checked uses of first_cpu elsewhere? Out of scope for this
> patch, of course.
I only looked at monitor.c so far, and that's the only spot that uses
this variable there.
But it seems like gdbstub.c has the same bug, too. If I start the "none"
machine and attach a remote gdb, QEMU segfaults here, too.
I've put this on my TODO-list... (I think it should be fixed with a
separate patch).
>> Signed-off-by: Thomas Huth <address@hidden>
>> ---
>> monitor.c | 29 +++++++++++++++++++++++++----
>> 1 file changed, 25 insertions(+), 4 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 0841d43..0103979 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1025,6 +1025,9 @@ int monitor_set_cpu(int cpu_index)
>> CPUState *mon_get_cpu(void)
>> {
>> if (!cur_mon->mon_cpu) {
>> + if (!first_cpu) {
>> + return NULL;
>> + }
>> monitor_set_cpu(first_cpu->cpu_index);
>> }
>> cpu_synchronize_state(cur_mon->mon_cpu);
>
> Why are the following dereferences safe?
>
> CPUArchState *mon_get_cpu_env(void)
> {
> return mon_get_cpu()->env_ptr;
> }
>
> int monitor_get_cpu_index(void)
> {
> return mon_get_cpu()->cpu_index;
> }
Oh, they are apparently not safe either. The HMP commands "nmi" and
"memsave", which use these functions, are crashing on the "none"
machine, too... I'll send a v2 of my patch to fix these, too ...
Thanks for the review!
Thomas