Skip to contentSkip to conversation

hmp: fix "dump-quest-memory" segfault (ppc)

PATCH Superseded Laurent Vivier 7 participants 16 messages Sep 11, 2017 – Sep 12, 2017

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hmp: fix "dump-quest-memory" segfault (ppc)

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

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.

---
 target/ppc/arch_dump.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
target/ppc/arch_dump.c +11 -6
index 8e9397aa58..dcb7b19950 100644
@@ -224,17 +224,22 @@ 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);
-
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;
 
-    if ((*pcc->interrupts_big_endian)(cpu)) {
-        info->d_endian = ELFDATA2MSB;
+    if (first_cpu) {
+        PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+        if ((*pcc->interrupts_big_endian)(cpu)) {
+            info->d_endian = ELFDATA2MSB;
+        } else {
+            info->d_endian = ELFDATA2LSB;
+        }
     } else {
-        info->d_endian = ELFDATA2LSB;
+        info->d_endian = ELFDATA2MSB;
     }
+
     /* 64KB is the max page size for pseries kernel */
     if (strncmp(object_get_typename(qdev_get_machine()),
                 "pseries-", 8) == 0) {
Signature · 1 line
2.13.5
#1 lore raw
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
#2 lore raw
Quoted 7 lines · Daniel P. Berrange
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

s390 works fine, but aarch64 needs the same kind of fix.

Thanks,
Laurent

#3 lore raw
In reply to Daniel P. Berrange
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

I can take a stab at fixing that, unless someone beats me to it.

#4 lore raw
Quoted 11 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

Quoted 8 lines

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
#5 lore raw
Quoted 19 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 18 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

Quoted 5 lines
Quoted 1 line

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

We do have - that's how we found this case; it's part of test-hmp.

Dave

Quoted 6 lines

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#6 lore raw
Quoted 20 lines · Dr. David Alan Gilbert
Quoted 19 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 18 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

We do have - that's how we found this case; it's part of test-hmp.

The test-hmp runs by default with 0 MB of memory, the problem can only be found with some memory added to the machine.

Perhaps we can simply update the test to add memory?

BTW, I'm not sure it is really useful to dump memory of a machine without CPU.

Laurent

#7 lore raw
Quoted 27 lines · Laurent Vivier
Quoted 26 lines · Dr. David Alan Gilbert
Quoted 24 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 11 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

Quoted 8 lines

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

Quoted 6 lines
Quoted 1 line

We do have - that's how we found this case; it's part of test-hmp.

The test-hmp runs by default with 0 MB of memory, the problem can only be found with some memory added to the machine.

Perhaps we can simply update the test to add memory?

Probably best to run it twice, 0MB and with say 2MB, as they're both fairly magic values.

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
#8 lore raw
Quoted 30 lines · Daniel P. Berrange
Quoted 29 lines · Laurent Vivier
Quoted 27 lines · Dr. David Alan Gilbert
Quoted 19 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 18 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

Quoted 5 lines
Quoted 1 line

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

We do have - that's how we found this case; it's part of test-hmp.

Quoted 7 lines
Quoted 4 lines

The test-hmp runs by default with 0 MB of memory, the problem can only be found with some memory added to the machine.

Perhaps we can simply update the test to add memory?

Probably best to run it twice, 0MB and with say 2MB, as they're both fairly magic values.

OK, I'm going to update the test.

Laurent

#9 lore raw
Quoted 34 lines · Laurent Vivier
Quoted 33 lines · Daniel P. Berrange
Quoted 32 lines · Laurent Vivier
Quoted 20 lines · Dr. David Alan Gilbert
Quoted 19 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 18 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

We do have - that's how we found this case; it's part of test-hmp.

The test-hmp runs by default with 0 MB of memory, the problem can only be found with some memory added to the machine.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

Perhaps we can simply update the test to add memory?

Probably best to run it twice, 0MB and with say 2MB, as they're both fairly magic values.

OK, I'm going to update the test.

Thanks! But please note that it is only the "none" machine which is started without memory and CPU by default - all the other boards should have a default set of memory and at least one CPU. So we should only add an additional tests for the "none" machine here.

Thomas

#10 lore raw
In reply to Laurent Vivier
Quoted 14 lines · On Mon, 11 Sep 2017 14:10:14 +0200 Laurent Vivier <[email protected]> wrote:
Quoted 13 lines · Dr. David Alan Gilbert
Quoted 11 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 5 lines · Cornelia Huck

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

I can take a stab at fixing that, unless someone beats me to it.

I wonder if someone wants to write a qtest job to run dump-guest-memory across all machine types, on all targets. Seems we have enough crashiness in this code to make it worthwhile to test

Quoted 6 lines
Quoted 1 line

We do have - that's how we found this case; it's part of test-hmp.

The test-hmp runs by default with 0 MB of memory, the problem can only be found with some memory added to the machine.

Perhaps we can simply update the test to add memory?

We have several combinations that can fail here... (cf. the problem with no memory and no filter above).

Quoted 3 lines

BTW, I'm not sure it is really useful to dump memory of a machine without CPU.

Even so, it should not segfault (and neither should dumping a guest with no memory, even if it doesn't make sense).

#11 lore raw
In reply to Cornelia Huck
Quoted 11 lines · Cornelia Huck
Quoted 7 lines · On Mon, 11 Sep 2017 12:06:15 +0100 "Daniel P. Berrange" <[email protected]>…
Quoted 2 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

What about all the other targets QEMU supports ? Have you checked if they are similarly affected, as we don't want to wait another 6 months to get a bug report that s390 or aarch64 crash in exactly the same way too.

Quoted 9 lines

This patch actually prompted me to check s390, and the mentioned command line works fine.

However, if we start a qemu with no guest memory defined and then call dump-guest-memory without filtering, we get a core dump instead of a guest dump (s390x or x86_64, machine none).

I can take a stab at fixing that, unless someone beats me to it.

I will update the test with this test case too.

Thanks,
Laurent

Reviewed-by on v1 1/1
#12 lore raw
In reply to Laurent Vivier
Quoted 6 lines · On Mon, 11 Sep 2017 13:00:37 +0200 Laurent Vivier <[email protected]> wrote:

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

Running QEMU with
qemu-system-ppc64 -M none -nographic -m 256
and executing

Quoted 7 lines
    dump-guest-memory /dev/null 0 8192

results in segfault

Fix by checking if we have CPU.

---

Quoted 36 lines
 target/ppc/arch_dump.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
target/ppc/arch_dump.c +11 -6
index 8e9397aa58..dcb7b19950 100644
@@ -224,17 +224,22 @@ 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);
-
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;
 
-    if ((*pcc->interrupts_big_endian)(cpu)) {
-        info->d_endian = ELFDATA2MSB;
+    if (first_cpu) {
+        PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+        if ((*pcc->interrupts_big_endian)(cpu)) {
+            info->d_endian = ELFDATA2MSB;
+        } else {
+            info->d_endian = ELFDATA2LSB;
+        }
     } else {
-        info->d_endian = ELFDATA2LSB;
+        info->d_endian = ELFDATA2MSB;
     }
+
     /* 64KB is the max page size for pseries kernel */
     if (strncmp(object_get_typename(qdev_get_machine()),
                 "pseries-", 8) == 0) {
Reviewed-by on v1 1/1
#13 lore raw
In reply to Laurent Vivier
Quoted 40 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

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.

---
 target/ppc/arch_dump.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
target/ppc/arch_dump.c +7 -5
index 8e9397aa58..dcb7b19950 100644
@@ -224,17 +224,22 @@ 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);
-
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;
 
-    if ((*pcc->interrupts_big_endian)(cpu)) {
-        info->d_endian = ELFDATA2MSB;
+    if (first_cpu) {
+        PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+        if ((*pcc->interrupts_big_endian)(cpu)) {
+            info->d_endian = ELFDATA2MSB;
+        } else {

target/ppc/arch_dump.c typedef struct NoteFuncDescStruct NoteFuncDesc; · line 245

Quoted 10 lines
target/ppc/arch_dump.c +4 -1
+            info->d_endian = ELFDATA2LSB;
+        }
     } else {
-        info->d_endian = ELFDATA2LSB;
+        info->d_endian = ELFDATA2MSB;
     }
+
     /* 64KB is the max page size for pseries kernel */
     if (strncmp(object_get_typename(qdev_get_machine()),
                 "pseries-", 8) == 0) {
#14 lore raw

----- Original Message -----

Quoted 52 lines

From: "Thomas Huth" <[email protected]>
To: "Laurent Vivier" <[email protected]>, [email protected]
Cc: "David Gibson" <[email protected]>, [email protected], "Dr . David Alan Gilbert" <[email protected]>, "Miroslav Rezanina" <[email protected]>
Sent: Monday, September 11, 2017 4:36:01 PM
Subject: Re: [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)

Quoted 44 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

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.

---
 target/ppc/arch_dump.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
target/ppc/arch_dump.c +10 -6
index 8e9397aa58..dcb7b19950 100644
@@ -224,17 +224,22 @@ 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);
-
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;
 
-    if ((*pcc->interrupts_big_endian)(cpu)) {
-        info->d_endian = ELFDATA2MSB;
+    if (first_cpu) {
+        PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+        if ((*pcc->interrupts_big_endian)(cpu)) {
+            info->d_endian = ELFDATA2MSB;
+        } else {
+            info->d_endian = ELFDATA2LSB;
+        }
     } else {
-        info->d_endian = ELFDATA2LSB;
+        info->d_endian = ELFDATA2MSB;
Quoted 9 lines

target/ppc/arch_dump.c typedef struct NoteFuncDescStruct NoteFuncDesc; · line 245

Quoted 6 lines
target/ppc/arch_dump.c +1 -0
     }
+
     /* 64KB is the max page size for pseries kernel */
     if (strncmp(object_get_typename(qdev_get_machine()),
                 "pseries-", 8) == 0) {

We need similar fix for aarch64 too.

Mirek

Signature · 2 lines
Miroslav Rezanina
Software Engineer - Virtualization Team
#15 lore raw
Quoted 21 lines · Miroslav Rezanina

----- Original Message -----

Quoted 18 lines

From: "Thomas Huth" <[email protected]>
To: "Laurent Vivier" <[email protected]>, [email protected]
Cc: "David Gibson" <[email protected]>, [email protected], "Dr . David Alan Gilbert" <[email protected]>, "Miroslav Rezanina" <[email protected]>
Sent: Monday, September 11, 2017 4:36:01 PM
Subject: Re: [PATCH] hmp: fix "dump-quest-memory" segfault (ppc)

Quoted 10 lines · Laurent Vivier

Commit fd5d23babf (hmp: fix "dump-quest-memory" segfault) fixes the problem for i386, do the same for ppc.

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.

[...]

Quoted 2 lines

We need similar fix for aarch64 too.

Yes, Laurent already posted a v2 which includes a patch for ARM, too:

https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02586.html

Thomas