On Tue, 11 Aug 2020 19:27:50 +0200
Laurent Vivier <lviv...@redhat.com> wrote:
> On 11/08/2020 18:36, Greg Kurz wrote:
> > On Tue, 11 Aug 2020 18:05:21 +0200
> > Laurent Vivier <lviv...@redhat.com> wrote:
> >
> >> On 11/08/2020 17:30, Greg Kurz wrote:
> >>> Since commit 61f20b9dc5b7 ("spapr_nvram: Pre-initialize the NVRAM to
> >>> support the -prom-env parameter"), pseries machines can pre-initialize
> >>> the "system" partition in the NVRAM with the data passed to all -prom-env
> >>> parameters on the QEMU command line.
> >>>
> >>> In this cases it is assumed that all the data fits in 64 KiB, but the user
> >>> can easily pass more and crash QEMU:
> >>>
> >>> $ qemu-system-ppc64 -M pseries $(for ((x=0;x<128;x++)); do \
> >>> echo -n " -prom-env "$(for ((y=0;y<1024;y++)); do echo -n x ; done) ; \
> >>> done) # this requires ~128 Kib
> >>> malloc(): corrupted top size
> >>> Aborted (core dumped)
> >>>
> >>> Call chrp_nvram_create_system_partition() first with its recently added
> >>> parameter dry_run set to false, to know the required size and allocate
> >>> the NVRAM buffer accordingly.
> >>>
> >>> Fixes: 61f20b9dc5b7 ("spapr_nvram: Pre-initialize the NVRAM to support
> >>> the -prom-env parameter")
> >>> Signed-off-by: Greg Kurz <gr...@kaod.org>
> >>> ---
> >>> hw/nvram/spapr_nvram.c | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
> >>> index 992b818d34e7..1b74bec6200a 100644
> >>> --- a/hw/nvram/spapr_nvram.c
> >>> +++ b/hw/nvram/spapr_nvram.c
> >>> @@ -165,6 +165,10 @@ static void spapr_nvram_realize(SpaprVioDevice *dev,
> >>> Error **errp)
> >>> if (ret < 0) {
> >>> return;
> >>> }
> >>> + } else if (nb_prom_envs > 0) {
> >>> + nvram->size = chrp_nvram_create_system_partition(NULL,
> >>> + MIN_NVRAM_SIZE
> >>> / 4,
> >>> + true);
> >>
> >> I think this will break the migration: the prom-env parameters can be on
> >> the source side without being on the dest side. And so the pram size
> >
> > Huh ? Migration mandates to have the same arguments on the command line,
> > and libvirt buys us that AFAIK or am I missing something ?
>
> It's not true for all the arguments: take for instance the blockdev, the
> chardev, the netdev, vnc, ... I think it's true only for parameters
> that define the devices, the machine. And until now for prom-env it was
> not the case, look at the migration-test test case.
>
arch_source = g_strdup_printf("-nodefaults "
"-prom-env 'use-nvramrc?=true' -prom-env "
"'nvramrc=hex .\" _\" begin %x %x "
"do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
"until'", end_address, start_address);
arch_target = g_strdup("");
Ok, my bad... I should have run that before posting.
So, I'll go the other way around and have QEMU to error out if the -prom-env
data doesn't fit in the default size (64 Kib).
> Thanks,
> Laurent
>
>
