Re: [PATCH v2 2/2] spapr/nvram: Error out if NVRAM cannot contain all -prom-env data

[John Snow]

On 8/12/20 1:08 PM, 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 true, in order to know the required size and fail
> gracefully if it's too small.
>
> Reported-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>

Fixes: 61f20b9dc5b7
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1867739

Thanks :)

> ---
>   hw/nvram/spapr_nvram.c |   15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
> index 992b818d34e7..c29d797ae1f0 100644
> --- a/hw/nvram/spapr_nvram.c
> +++ b/hw/nvram/spapr_nvram.c
> @@ -145,6 +145,7 @@ static void rtas_nvram_store(PowerPCCPU *cpu, 
> SpaprMachineState *spapr,
>   
>   static void spapr_nvram_realize(SpaprVioDevice *dev, Error **errp)
>   {
> +    ERRP_GUARD();
>       SpaprNvram *nvram = VIO_SPAPR_NVRAM(dev);
>       int ret;
>   
> @@ -187,6 +188,20 @@ static void spapr_nvram_realize(SpaprVioDevice *dev, Error **errp)
>               return;
>           }
>       } else if (nb_prom_envs > 0) {
> +        int len = chrp_nvram_create_system_partition(nvram->buf,
> +                                                     MIN_NVRAM_SIZE / 4,
> +                                                     true);
> +
> +        /* Check the partition is large enough for all the -prom-env data */
> +        if (nvram->size < len) {
> +            error_setg(errp, "-prom-env data requires %d bytes but spapr-nvram 
> "
> +                       "is only %d bytes in size", len, nvram->size);
> +            error_append_hint(errp,
> +                              "Try to pass %d less bytes to -prom-env.\n",
> +                              len - nvram->size);
> +            return;
> +        }
> +
>           /* Create a system partition to pass the -prom-env variables */
>           chrp_nvram_create_system_partition(nvram->buf, MIN_NVRAM_SIZE / 4,
>                                              false);