diff mbox series

[v3,1/3] hmp: fix "dump-quest-memory" segfault (ppc)

Message ID 20170912140149.7692-2-lvivier@redhat.com
State New
Headers show
Series hmp: fix "dump-quest-memory" segfault | expand

Commit Message

Laurent Vivier Sept. 12, 2017, 2:01 p.m. UTC
Running QEMU with
    qemu-system-ppc64 -M none -nographic -m 256
and executing
    dump-guest-memory /dev/null 0 8192
results in segfault

Fix by checking if we have CPU, and exit with
error if there is no CPU:

    (qemu) dump-guest-memory /dev/null
    this feature or command is not currently supported

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target/ppc/arch_dump.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Thomas Huth Sept. 12, 2017, 2:10 p.m. UTC | #1
On 12.09.2017 16:01, Laurent Vivier wrote:
> Running QEMU with
>     qemu-system-ppc64 -M none -nographic -m 256
> and executing
>     dump-guest-memory /dev/null 0 8192
> results in segfault
> 
> Fix by checking if we have CPU, and exit with
> error if there is no CPU:
> 
>     (qemu) dump-guest-memory /dev/null
>     this feature or command is not currently supported
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  target/ppc/arch_dump.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 8e9397aa58..95b9ab6f29 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
>  int cpu_get_dump_info(ArchDumpInfo *info,
>                        const struct GuestPhysBlockList *guest_phys_blocks)
>  {
> -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +    PowerPCCPU *cpu;
> +    PowerPCCPUClass *pcc;
> +
> +    if (first_cpu == NULL) {
> +        return -1;
> +    }
> +
> +    cpu = POWERPC_CPU(first_cpu);
> +    pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
>      info->d_machine = PPC_ELF_MACHINE;
>      info->d_class = ELFCLASS;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>
Greg Kurz Sept. 12, 2017, 2:48 p.m. UTC | #2
On Tue, 12 Sep 2017 16:01:47 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Running QEMU with
>     qemu-system-ppc64 -M none -nographic -m 256
> and executing
>     dump-guest-memory /dev/null 0 8192
> results in segfault
> 
> Fix by checking if we have CPU, and exit with
> error if there is no CPU:
> 
>     (qemu) dump-guest-memory /dev/null
>     this feature or command is not currently supported
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  target/ppc/arch_dump.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 8e9397aa58..95b9ab6f29 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
>  int cpu_get_dump_info(ArchDumpInfo *info,
>                        const struct GuestPhysBlockList *guest_phys_blocks)
>  {
> -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +    PowerPCCPU *cpu;
> +    PowerPCCPUClass *pcc;
> +
> +    if (first_cpu == NULL) {
> +        return -1;
> +    }
> +
> +    cpu = POWERPC_CPU(first_cpu);
> +    pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
>      info->d_machine = PPC_ELF_MACHINE;
>      info->d_class = ELFCLASS;
David Gibson Sept. 13, 2017, 5:35 a.m. UTC | #3
On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
> Running QEMU with
>     qemu-system-ppc64 -M none -nographic -m 256
> and executing
>     dump-guest-memory /dev/null 0 8192
> results in segfault
> 
> Fix by checking if we have CPU, and exit with
> error if there is no CPU:
> 
>     (qemu) dump-guest-memory /dev/null
>     this feature or command is not currently supported
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
of the series though.

> ---
>  target/ppc/arch_dump.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 8e9397aa58..95b9ab6f29 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
>  int cpu_get_dump_info(ArchDumpInfo *info,
>                        const struct GuestPhysBlockList *guest_phys_blocks)
>  {
> -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +    PowerPCCPU *cpu;
> +    PowerPCCPUClass *pcc;
> +
> +    if (first_cpu == NULL) {
> +        return -1;
> +    }
> +
> +    cpu = POWERPC_CPU(first_cpu);
> +    pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
>      info->d_machine = PPC_ELF_MACHINE;
>      info->d_class = ELFCLASS;
Dr. David Alan Gilbert Sept. 13, 2017, 12:44 p.m. UTC | #4
* David Gibson (david@gibson.dropbear.id.au) wrote:
> On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
> > Running QEMU with
> >     qemu-system-ppc64 -M none -nographic -m 256
> > and executing
> >     dump-guest-memory /dev/null 0 8192
> > results in segfault
> > 
> > Fix by checking if we have CPU, and exit with
> > error if there is no CPU:
> > 
> >     (qemu) dump-guest-memory /dev/null
> >     this feature or command is not currently supported
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
> of the series though.

I can take the whole set through HMP.

Dave

> > ---
> >  target/ppc/arch_dump.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> > index 8e9397aa58..95b9ab6f29 100644
> > --- a/target/ppc/arch_dump.c
> > +++ b/target/ppc/arch_dump.c
> > @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
> >  int cpu_get_dump_info(ArchDumpInfo *info,
> >                        const struct GuestPhysBlockList *guest_phys_blocks)
> >  {
> > -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > +    PowerPCCPU *cpu;
> > +    PowerPCCPUClass *pcc;
> > +
> > +    if (first_cpu == NULL) {
> > +        return -1;
> > +    }
> > +
> > +    cpu = POWERPC_CPU(first_cpu);
> > +    pcc = POWERPC_CPU_GET_CLASS(cpu);
> >  
> >      info->d_machine = PPC_ELF_MACHINE;
> >      info->d_class = ELFCLASS;
> 
> -- 
> David Gibson			| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> 				| _way_ _around_!
> http://www.ozlabs.org/~dgibson


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Cornelia Huck Sept. 13, 2017, 12:48 p.m. UTC | #5
On Wed, 13 Sep 2017 13:44:57 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * David Gibson (david@gibson.dropbear.id.au) wrote:
> > On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:  
> > > Running QEMU with
> > >     qemu-system-ppc64 -M none -nographic -m 256
> > > and executing
> > >     dump-guest-memory /dev/null 0 8192
> > > results in segfault
> > > 
> > > Fix by checking if we have CPU, and exit with
> > > error if there is no CPU:
> > > 
> > >     (qemu) dump-guest-memory /dev/null
> > >     this feature or command is not currently supported
> > > 
> > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> > 
> > Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
> > of the series though.  
> 
> I can take the whole set through HMP.

If you do so, you should probably take "dump: do not dump non-existent
guest memory" as well.
Laurent Vivier Sept. 13, 2017, 12:49 p.m. UTC | #6
On 13/09/2017 14:44, Dr. David Alan Gilbert wrote:
> * David Gibson (david@gibson.dropbear.id.au) wrote:
>> On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
>>> Running QEMU with
>>>     qemu-system-ppc64 -M none -nographic -m 256
>>> and executing
>>>     dump-guest-memory /dev/null 0 8192
>>> results in segfault
>>>
>>> Fix by checking if we have CPU, and exit with
>>> error if there is no CPU:
>>>
>>>     (qemu) dump-guest-memory /dev/null
>>>     this feature or command is not currently supported
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>
>> Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
>> of the series though.
> 
> I can take the whole set through HMP.

Could you update the comment in last patch as asked by Cornelia?
(PATCH v3 3/3] tests/hmp: test "none" machine with memory)

Thanks,
Laurent

> 
> Dave
> 
>>> ---
>>>  target/ppc/arch_dump.c | 11 +++++++++--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
>>> index 8e9397aa58..95b9ab6f29 100644
>>> --- a/target/ppc/arch_dump.c
>>> +++ b/target/ppc/arch_dump.c
>>> @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
>>>  int cpu_get_dump_info(ArchDumpInfo *info,
>>>                        const struct GuestPhysBlockList *guest_phys_blocks)
>>>  {
>>> -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>>> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>>> +    PowerPCCPU *cpu;
>>> +    PowerPCCPUClass *pcc;
>>> +
>>> +    if (first_cpu == NULL) {
>>> +        return -1;
>>> +    }
>>> +
>>> +    cpu = POWERPC_CPU(first_cpu);
>>> +    pcc = POWERPC_CPU_GET_CLASS(cpu);
>>>  
>>>      info->d_machine = PPC_ELF_MACHINE;
>>>      info->d_class = ELFCLASS;
>>
>> -- 
>> David Gibson			| I'll have my music baroque, and my code
>> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
>> 				| _way_ _around_!
>> http://www.ozlabs.org/~dgibson
> 
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
David Gibson Sept. 13, 2017, 12:54 p.m. UTC | #7
On Wed, Sep 13, 2017 at 01:44:57PM +0100, Dr. David Alan Gilbert wrote:
> * David Gibson (david@gibson.dropbear.id.au) wrote:
> > On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
> > > Running QEMU with
> > >     qemu-system-ppc64 -M none -nographic -m 256
> > > and executing
> > >     dump-guest-memory /dev/null 0 8192
> > > results in segfault
> > > 
> > > Fix by checking if we have CPU, and exit with
> > > error if there is no CPU:
> > > 
> > >     (qemu) dump-guest-memory /dev/null
> > >     this feature or command is not currently supported
> > > 
> > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > 
> > Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
> > of the series though.
> 
> I can take the whole set through HMP.

Great, thanks.

> 
> Dave
> 
> > > ---
> > >  target/ppc/arch_dump.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> > > index 8e9397aa58..95b9ab6f29 100644
> > > --- a/target/ppc/arch_dump.c
> > > +++ b/target/ppc/arch_dump.c
> > > @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
> > >  int cpu_get_dump_info(ArchDumpInfo *info,
> > >                        const struct GuestPhysBlockList *guest_phys_blocks)
> > >  {
> > > -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> > > -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > > +    PowerPCCPU *cpu;
> > > +    PowerPCCPUClass *pcc;
> > > +
> > > +    if (first_cpu == NULL) {
> > > +        return -1;
> > > +    }
> > > +
> > > +    cpu = POWERPC_CPU(first_cpu);
> > > +    pcc = POWERPC_CPU_GET_CLASS(cpu);
> > >  
> > >      info->d_machine = PPC_ELF_MACHINE;
> > >      info->d_class = ELFCLASS;
> > 
> 
>
Dr. David Alan Gilbert Sept. 13, 2017, 1:19 p.m. UTC | #8
* Laurent Vivier (lvivier@redhat.com) wrote:
> On 13/09/2017 14:44, Dr. David Alan Gilbert wrote:
> > * David Gibson (david@gibson.dropbear.id.au) wrote:
> >> On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
> >>> Running QEMU with
> >>>     qemu-system-ppc64 -M none -nographic -m 256
> >>> and executing
> >>>     dump-guest-memory /dev/null 0 8192
> >>> results in segfault
> >>>
> >>> Fix by checking if we have CPU, and exit with
> >>> error if there is no CPU:
> >>>
> >>>     (qemu) dump-guest-memory /dev/null
> >>>     this feature or command is not currently supported
> >>>
> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>
> >> Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
> >> of the series though.
> > 
> > I can take the whole set through HMP.
> 
> Could you update the comment in last patch as asked by Cornelia?
> (PATCH v3 3/3] tests/hmp: test "none" machine with memory)

Sure, do we understand what the error that patchew caught was?

Dave

> Thanks,
> Laurent
> 
> > 
> > Dave
> > 
> >>> ---
> >>>  target/ppc/arch_dump.c | 11 +++++++++--
> >>>  1 file changed, 9 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> >>> index 8e9397aa58..95b9ab6f29 100644
> >>> --- a/target/ppc/arch_dump.c
> >>> +++ b/target/ppc/arch_dump.c
> >>> @@ -224,8 +224,15 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
> >>>  int cpu_get_dump_info(ArchDumpInfo *info,
> >>>                        const struct GuestPhysBlockList *guest_phys_blocks)
> >>>  {
> >>> -    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> >>> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> >>> +    PowerPCCPU *cpu;
> >>> +    PowerPCCPUClass *pcc;
> >>> +
> >>> +    if (first_cpu == NULL) {
> >>> +        return -1;
> >>> +    }
> >>> +
> >>> +    cpu = POWERPC_CPU(first_cpu);
> >>> +    pcc = POWERPC_CPU_GET_CLASS(cpu);
> >>>  
> >>>      info->d_machine = PPC_ELF_MACHINE;
> >>>      info->d_class = ELFCLASS;
> >>
> >> -- 
> >> David Gibson			| I'll have my music baroque, and my code
> >> david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
> >> 				| _way_ _around_!
> >> http://www.ozlabs.org/~dgibson
> > 
> > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Laurent Vivier Sept. 13, 2017, 1:23 p.m. UTC | #9
On 13/09/2017 15:19, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (lvivier@redhat.com) wrote:
>> On 13/09/2017 14:44, Dr. David Alan Gilbert wrote:
>>> * David Gibson (david@gibson.dropbear.id.au) wrote:
>>>> On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
>>>>> Running QEMU with
>>>>>     qemu-system-ppc64 -M none -nographic -m 256
>>>>> and executing
>>>>>     dump-guest-memory /dev/null 0 8192
>>>>> results in segfault
>>>>>
>>>>> Fix by checking if we have CPU, and exit with
>>>>> error if there is no CPU:
>>>>>
>>>>>     (qemu) dump-guest-memory /dev/null
>>>>>     this feature or command is not currently supported
>>>>>
>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>
>>>> Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
>>>> of the series though.
>>>
>>> I can take the whole set through HMP.
>>
>> Could you update the comment in last patch as asked by Cornelia?
>> (PATCH v3 3/3] tests/hmp: test "none" machine with memory)
> 
> Sure, do we understand what the error that patchew caught was?

We need the fix from Cornelia, otherwise the new test fails.

Do you want I resend the series including the fix from Cornelia to be
sure patchew works?

Thanks,
Laurent
Dr. David Alan Gilbert Sept. 13, 2017, 1:40 p.m. UTC | #10
* Laurent Vivier (lvivier@redhat.com) wrote:
> On 13/09/2017 15:19, Dr. David Alan Gilbert wrote:
> > * Laurent Vivier (lvivier@redhat.com) wrote:
> >> On 13/09/2017 14:44, Dr. David Alan Gilbert wrote:
> >>> * David Gibson (david@gibson.dropbear.id.au) wrote:
> >>>> On Tue, Sep 12, 2017 at 04:01:47PM +0200, Laurent Vivier wrote:
> >>>>> Running QEMU with
> >>>>>     qemu-system-ppc64 -M none -nographic -m 256
> >>>>> and executing
> >>>>>     dump-guest-memory /dev/null 0 8192
> >>>>> results in segfault
> >>>>>
> >>>>> Fix by checking if we have CPU, and exit with
> >>>>> error if there is no CPU:
> >>>>>
> >>>>>     (qemu) dump-guest-memory /dev/null
> >>>>>     this feature or command is not currently supported
> >>>>>
> >>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>>
> >>>> Applied to ppc-for-2.11.  I'm not really sure what to do with the rest
> >>>> of the series though.
> >>>
> >>> I can take the whole set through HMP.
> >>
> >> Could you update the comment in last patch as asked by Cornelia?
> >> (PATCH v3 3/3] tests/hmp: test "none" machine with memory)
> > 
> > Sure, do we understand what the error that patchew caught was?
> 
> We need the fix from Cornelia, otherwise the new test fails.
> 
> Do you want I resend the series including the fix from Cornelia to be
> sure patchew works?

Yes, that's best because I think you're saying we need to put
Cornelia's fix in before your 3/3 to ensure it's bisectable.

Dave

> Thanks,
> Laurent
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index 8e9397aa58..95b9ab6f29 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -224,8 +224,15 @@  typedef struct NoteFuncDescStruct NoteFuncDesc;
 int cpu_get_dump_info(ArchDumpInfo *info,
                       const struct GuestPhysBlockList *guest_phys_blocks)
 {
-    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
-    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    PowerPCCPU *cpu;
+    PowerPCCPUClass *pcc;
+
+    if (first_cpu == NULL) {
+        return -1;
+    }
+
+    cpu = POWERPC_CPU(first_cpu);
+    pcc = POWERPC_CPU_GET_CLASS(cpu);
 
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;