[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[qemu-xen stable-4.14] nvram: Exit QEMU if NVRAM cannot contain all -prom-env data



commit ebf5b3946e49bc10eef1234753f3fac3ac14d3e5
Author:     Greg Kurz <groug@xxxxxxxx>
AuthorDate: Fri Aug 14 01:12:19 2020 +0200
Commit:     Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
CommitDate: Thu Sep 10 13:09:51 2020 -0500

    nvram: Exit QEMU if NVRAM cannot contain all -prom-env data
    
    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 case 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 " ; printf "%0.sx" {1..1024}; \
      done) # this requires ~128 Kib
    malloc(): corrupted top size
    Aborted (core dumped)
    
    This happens because we don't check if all the prom-env data fits in
    the NVRAM and chrp_nvram_set_var() happily memcpy() it passed the
    buffer.
    
    This crash affects basically all ppc/ppc64 machine types that use -prom-env:
    - pseries (all versions)
    - g3beige
    - mac99
    
    and also sparc/sparc64 machine types:
    - LX
    - SPARCClassic
    - SPARCbook
    - SS-10
    - SS-20
    - SS-4
    - SS-5
    - SS-600MP
    - Voyager
    - sun4u
    - sun4v
    
    Add a max_len argument to chrp_nvram_create_system_partition() so that
    it can check the available size before writing to memory.
    
    Since NVRAM is populated at machine init, it seems reasonable to consider
    this error as fatal. So, instead of reporting an error when we detect that
    the NVRAM is too small and adapt all machine types to handle it, we simply
    exit QEMU in all cases. This is still better than crashing. If someone
    wants another behavior, I guess this can be reworked later.
    
    Tested with:
    
    $ yes q | \
      (for arch in ppc ppc64 sparc sparc64; do \
           echo == $arch ==; \
           qemu=${arch}-softmmu/qemu-system-$arch; \
           for mach in $($qemu -M help | awk '! /^Supported/ { print $1 }'); do 
\
               echo $mach; \
               $qemu -M $mach -monitor stdio -nodefaults -nographic \
               $(for ((x=0;x<128;x++)); do \
                     echo -n " -prom-env " ; printf "%0.sx" {1..1024}; \
                 done) >/dev/null; \
            done; echo; \
       done)
    
    Without the patch, affected machine types cause QEMU to report some
    memory corruption and crash:
    
    malloc(): corrupted top size
    
    free(): invalid size
    
    *** stack smashing detected ***: terminated
    
    With the patch, QEMU prints the following message and exits:
    
    NVRAM is too small. Try to pass less data to -prom-env
    
    It seems that the conditions for the crash have always existed, but it
    affects pseries, the machine type I care for, since commit 61f20b9dc5b7
    only.
    
    Fixes: 61f20b9dc5b7 ("spapr_nvram: Pre-initialize the NVRAM to support the 
-prom-env parameter")
    RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1867739
    Reported-by: John Snow <jsnow@xxxxxxxxxx>
    Reviewed-by: Laurent Vivier <laurent@xxxxxxxxx>
    Signed-off-by: Greg Kurz <groug@xxxxxxxx>
    Message-Id: <159736033937.350502.12402444542194031035.stgit@xxxxxxxxx>
    Signed-off-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
    (cherry picked from commit 37035df51eaabb8d26b71da75b88a1c6727de8fa)
    Signed-off-by: Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
---
 hw/nvram/chrp_nvram.c         | 24 +++++++++++++++++++++---
 hw/nvram/mac_nvram.c          |  2 +-
 hw/nvram/spapr_nvram.c        |  3 ++-
 hw/sparc/sun4m.c              |  2 +-
 hw/sparc64/sun4u.c            |  2 +-
 include/hw/nvram/chrp_nvram.h |  3 ++-
 6 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/hw/nvram/chrp_nvram.c b/hw/nvram/chrp_nvram.c
index d969f26704..d4d10a7c03 100644
--- a/hw/nvram/chrp_nvram.c
+++ b/hw/nvram/chrp_nvram.c
@@ -21,14 +21,21 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 #include "hw/nvram/chrp_nvram.h"
 #include "sysemu/sysemu.h"
 
-static int chrp_nvram_set_var(uint8_t *nvram, int addr, const char *str)
+static int chrp_nvram_set_var(uint8_t *nvram, int addr, const char *str,
+                              int max_len)
 {
     int len;
 
     len = strlen(str) + 1;
+
+    if (max_len < len) {
+        return -1;
+    }
+
     memcpy(&nvram[addr], str, len);
 
     return addr + len;
@@ -38,19 +45,26 @@ static int chrp_nvram_set_var(uint8_t *nvram, int addr, 
const char *str)
  * Create a "system partition", used for the Open Firmware
  * environment variables.
  */
-int chrp_nvram_create_system_partition(uint8_t *data, int min_len)
+int chrp_nvram_create_system_partition(uint8_t *data, int min_len, int max_len)
 {
     ChrpNvramPartHdr *part_header;
     unsigned int i;
     int end;
 
+    if (max_len < sizeof(*part_header)) {
+        goto fail;
+    }
+
     part_header = (ChrpNvramPartHdr *)data;
     part_header->signature = CHRP_NVPART_SYSTEM;
     pstrcpy(part_header->name, sizeof(part_header->name), "system");
 
     end = sizeof(ChrpNvramPartHdr);
     for (i = 0; i < nb_prom_envs; i++) {
-        end = chrp_nvram_set_var(data, end, prom_envs[i]);
+        end = chrp_nvram_set_var(data, end, prom_envs[i], max_len - end);
+        if (end == -1) {
+            goto fail;
+        }
     }
 
     /* End marker */
@@ -65,6 +79,10 @@ int chrp_nvram_create_system_partition(uint8_t *data, int 
min_len)
     chrp_nvram_finish_partition(part_header, end);
 
     return end;
+
+fail:
+    error_report("NVRAM is too small. Try to pass less data to -prom-env");
+    exit(EXIT_FAILURE);
 }
 
 /**
diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
index 2e8a1e3812..1c44ee19e1 100644
--- a/hw/nvram/mac_nvram.c
+++ b/hw/nvram/mac_nvram.c
@@ -152,7 +152,7 @@ static void pmac_format_nvram_partition_of(MacIONVRAMState 
*nvr, int off,
 
     /* OpenBIOS nvram variables partition */
     sysp_end = chrp_nvram_create_system_partition(&nvr->data[off],
-                                                  DEF_SYSTEM_SIZE) + off;
+                                                  DEF_SYSTEM_SIZE, len) + off;
 
     /* Free space partition */
     chrp_nvram_create_free_partition(&nvr->data[sysp_end], len - sysp_end);
diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index 15d08281d4..386513499f 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -188,7 +188,8 @@ static void spapr_nvram_realize(SpaprVioDevice *dev, Error 
**errp)
         }
     } else if (nb_prom_envs > 0) {
         /* Create a system partition to pass the -prom-env variables */
-        chrp_nvram_create_system_partition(nvram->buf, MIN_NVRAM_SIZE / 4);
+        chrp_nvram_create_system_partition(nvram->buf, MIN_NVRAM_SIZE / 4,
+                                           nvram->size);
         chrp_nvram_create_free_partition(&nvram->buf[MIN_NVRAM_SIZE / 4],
                                          nvram->size - MIN_NVRAM_SIZE / 4);
     }
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 36ee1a0a3d..fa77e2a5e2 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -142,7 +142,7 @@ static void nvram_init(Nvram *nvram, uint8_t *macaddr,
     memset(image, '\0', sizeof(image));
 
     /* OpenBIOS nvram variables partition */
-    sysp_end = chrp_nvram_create_system_partition(image, 0);
+    sysp_end = chrp_nvram_create_system_partition(image, 0, 0x1fd0);
 
     /* Free space partition */
     chrp_nvram_create_free_partition(&image[sysp_end], 0x1fd0 - sysp_end);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 6abfcb30f8..9dcb9a4bc8 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -136,7 +136,7 @@ static int sun4u_NVRAM_set_params(Nvram *nvram, uint16_t 
NVRAM_size,
     memset(image, '\0', sizeof(image));
 
     /* OpenBIOS nvram variables partition */
-    sysp_end = chrp_nvram_create_system_partition(image, 0);
+    sysp_end = chrp_nvram_create_system_partition(image, 0, 0x1fd0);
 
     /* Free space partition */
     chrp_nvram_create_free_partition(&image[sysp_end], 0x1fd0 - sysp_end);
diff --git a/include/hw/nvram/chrp_nvram.h b/include/hw/nvram/chrp_nvram.h
index 09941a9be4..4a0f5c21b8 100644
--- a/include/hw/nvram/chrp_nvram.h
+++ b/include/hw/nvram/chrp_nvram.h
@@ -50,7 +50,8 @@ chrp_nvram_finish_partition(ChrpNvramPartHdr *header, 
uint32_t size)
     header->checksum = sum & 0xff;
 }
 
-int chrp_nvram_create_system_partition(uint8_t *data, int min_len);
+/* chrp_nvram_create_system_partition() failure is fatal */
+int chrp_nvram_create_system_partition(uint8_t *data, int min_len, int 
max_len);
 int chrp_nvram_create_free_partition(uint8_t *data, int len);
 
 #endif
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#stable-4.14



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.