diff mbox series

pmu: fix pmu vmstate subsection list

Message ID 20211116150837.169291-1-lvivier@redhat.com (mailing list archive)
State New, archived
Headers show
Series pmu: fix pmu vmstate subsection list | expand

Commit Message

Laurent Vivier Nov. 16, 2021, 3:08 p.m. UTC
The subsection is not closed by a NULL marker so this can trigger
a segfault when the pmu vmstate is saved.

This can be easily shown with:

  $ ./qemu-system-ppc64  -dump-vmstate vmstate.json
  Segmentation fault (core dumped)

Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
Cc: mark.cave-ayland@ilande.co.uk
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/misc/macio/pmu.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Greg Kurz Nov. 16, 2021, 3:20 p.m. UTC | #1
On Tue, 16 Nov 2021 16:08:37 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> The subsection is not closed by a NULL marker so this can trigger
> a segfault when the pmu vmstate is saved.
> 
> This can be easily shown with:
> 
>   $ ./qemu-system-ppc64  -dump-vmstate vmstate.json
>   Segmentation fault (core dumped)
> 
> Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
> Cc: mark.cave-ayland@ilande.co.uk
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

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

>  hw/misc/macio/pmu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 4ad4f50e08c3..eb39c64694aa 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
>      },
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_pmu_adb,
> +        NULL
>      }
>  };
>  

This fix is so obvious that I guess you could carry it through the
trivial tree IMHO.
Philippe Mathieu-Daudé Nov. 16, 2021, 3:44 p.m. UTC | #2
On 11/16/21 16:08, Laurent Vivier wrote:
> The subsection is not closed by a NULL marker so this can trigger
> a segfault when the pmu vmstate is saved.
> 
> This can be easily shown with:
> 
>   $ ./qemu-system-ppc64  -dump-vmstate vmstate.json
>   Segmentation fault (core dumped)
> 
> Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
> Cc: mark.cave-ayland@ilande.co.uk
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/misc/macio/pmu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 4ad4f50e08c3..eb39c64694aa 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
>      },
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_pmu_adb,
> +        NULL
>      }
>  };

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

BTW I ran 'git grep -W -F .subsections' and couldn't find other
occurrence.
Mark Cave-Ayland Nov. 16, 2021, 7:50 p.m. UTC | #3
On 16/11/2021 15:08, Laurent Vivier wrote:

> The subsection is not closed by a NULL marker so this can trigger
> a segfault when the pmu vmstate is saved.
> 
> This can be easily shown with:
> 
>    $ ./qemu-system-ppc64  -dump-vmstate vmstate.json
>    Segmentation fault (core dumped)
> 
> Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
> Cc: mark.cave-ayland@ilande.co.uk
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   hw/misc/macio/pmu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 4ad4f50e08c3..eb39c64694aa 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
>       },
>       .subsections = (const VMStateDescription * []) {
>           &vmstate_pmu_adb,
> +        NULL
>       }
>   };

Eeek. Good spot, looks like this bug has been around for some time:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
Cédric Le Goater Nov. 17, 2021, 11:03 a.m. UTC | #4
>> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
>> index 4ad4f50e08c3..eb39c64694aa 100644
>> --- a/hw/misc/macio/pmu.c
>> +++ b/hw/misc/macio/pmu.c
>> @@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
>>       },
>>       .subsections = (const VMStateDescription * []) {
>>           &vmstate_pmu_adb,
>> +        NULL
>>       }
>>   };
>>   
> 
> This fix is so obvious that I guess you could carry it through the
> trivial tree IMHO.

I don't have anything queued for ppc yet but anyhow I can send a PR
at the end of the week if trivial doesn't.

Thanks,

C.
Cédric Le Goater Nov. 17, 2021, 6:13 p.m. UTC | #5
On 11/16/21 16:08, Laurent Vivier wrote:
> The subsection is not closed by a NULL marker so this can trigger
> a segfault when the pmu vmstate is saved.
> 
> This can be easily shown with:
> 
>    $ ./qemu-system-ppc64  -dump-vmstate vmstate.json
>    Segmentation fault (core dumped)
> 
> Fixes: d811d61fbc6c ("mac_newworld: add PMU device")
> Cc: mark.cave-ayland@ilande.co.uk
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Queued for 6.2

Thanks,

C.


> ---
>   hw/misc/macio/pmu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 4ad4f50e08c3..eb39c64694aa 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -718,6 +718,7 @@ static const VMStateDescription vmstate_pmu = {
>       },
>       .subsections = (const VMStateDescription * []) {
>           &vmstate_pmu_adb,
> +        NULL
>       }
>   };
>   
>
diff mbox series

Patch

diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 4ad4f50e08c3..eb39c64694aa 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -718,6 +718,7 @@  static const VMStateDescription vmstate_pmu = {
     },
     .subsections = (const VMStateDescription * []) {
         &vmstate_pmu_adb,
+        NULL
     }
 };