Skip to content

Commit ebf5b39

Browse files
gkurzmdroth
authored andcommitted
nvram: Exit QEMU if NVRAM cannot contain all -prom-env data
Since commit 61f20b9 ("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 61f20b9 only. Fixes: 61f20b9 ("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@redhat.com> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <159736033937.350502.12402444542194031035.stgit@bahia.lan> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> (cherry picked from commit 37035df) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
1 parent f2fd655 commit ebf5b39

6 files changed

Lines changed: 28 additions & 8 deletions

File tree

hw/nvram/chrp_nvram.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,21 @@
2121

2222
#include "qemu/osdep.h"
2323
#include "qemu/cutils.h"
24+
#include "qemu/error-report.h"
2425
#include "hw/nvram/chrp_nvram.h"
2526
#include "sysemu/sysemu.h"
2627

27-
static int chrp_nvram_set_var(uint8_t *nvram, int addr, const char *str)
28+
static int chrp_nvram_set_var(uint8_t *nvram, int addr, const char *str,
29+
int max_len)
2830
{
2931
int len;
3032

3133
len = strlen(str) + 1;
34+
35+
if (max_len < len) {
36+
return -1;
37+
}
38+
3239
memcpy(&nvram[addr], str, len);
3340

3441
return addr + len;
@@ -38,19 +45,26 @@ static int chrp_nvram_set_var(uint8_t *nvram, int addr, const char *str)
3845
* Create a "system partition", used for the Open Firmware
3946
* environment variables.
4047
*/
41-
int chrp_nvram_create_system_partition(uint8_t *data, int min_len)
48+
int chrp_nvram_create_system_partition(uint8_t *data, int min_len, int max_len)
4249
{
4350
ChrpNvramPartHdr *part_header;
4451
unsigned int i;
4552
int end;
4653

54+
if (max_len < sizeof(*part_header)) {
55+
goto fail;
56+
}
57+
4758
part_header = (ChrpNvramPartHdr *)data;
4859
part_header->signature = CHRP_NVPART_SYSTEM;
4960
pstrcpy(part_header->name, sizeof(part_header->name), "system");
5061

5162
end = sizeof(ChrpNvramPartHdr);
5263
for (i = 0; i < nb_prom_envs; i++) {
53-
end = chrp_nvram_set_var(data, end, prom_envs[i]);
64+
end = chrp_nvram_set_var(data, end, prom_envs[i], max_len - end);
65+
if (end == -1) {
66+
goto fail;
67+
}
5468
}
5569

5670
/* End marker */
@@ -65,6 +79,10 @@ int chrp_nvram_create_system_partition(uint8_t *data, int min_len)
6579
chrp_nvram_finish_partition(part_header, end);
6680

6781
return end;
82+
83+
fail:
84+
error_report("NVRAM is too small. Try to pass less data to -prom-env");
85+
exit(EXIT_FAILURE);
6886
}
6987

7088
/**

hw/nvram/mac_nvram.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ static void pmac_format_nvram_partition_of(MacIONVRAMState *nvr, int off,
152152

153153
/* OpenBIOS nvram variables partition */
154154
sysp_end = chrp_nvram_create_system_partition(&nvr->data[off],
155-
DEF_SYSTEM_SIZE) + off;
155+
DEF_SYSTEM_SIZE, len) + off;
156156

157157
/* Free space partition */
158158
chrp_nvram_create_free_partition(&nvr->data[sysp_end], len - sysp_end);

hw/nvram/spapr_nvram.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ static void spapr_nvram_realize(SpaprVioDevice *dev, Error **errp)
188188
}
189189
} else if (nb_prom_envs > 0) {
190190
/* Create a system partition to pass the -prom-env variables */
191-
chrp_nvram_create_system_partition(nvram->buf, MIN_NVRAM_SIZE / 4);
191+
chrp_nvram_create_system_partition(nvram->buf, MIN_NVRAM_SIZE / 4,
192+
nvram->size);
192193
chrp_nvram_create_free_partition(&nvram->buf[MIN_NVRAM_SIZE / 4],
193194
nvram->size - MIN_NVRAM_SIZE / 4);
194195
}

hw/sparc/sun4m.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ static void nvram_init(Nvram *nvram, uint8_t *macaddr,
142142
memset(image, '\0', sizeof(image));
143143

144144
/* OpenBIOS nvram variables partition */
145-
sysp_end = chrp_nvram_create_system_partition(image, 0);
145+
sysp_end = chrp_nvram_create_system_partition(image, 0, 0x1fd0);
146146

147147
/* Free space partition */
148148
chrp_nvram_create_free_partition(&image[sysp_end], 0x1fd0 - sysp_end);

hw/sparc64/sun4u.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ static int sun4u_NVRAM_set_params(Nvram *nvram, uint16_t NVRAM_size,
136136
memset(image, '\0', sizeof(image));
137137

138138
/* OpenBIOS nvram variables partition */
139-
sysp_end = chrp_nvram_create_system_partition(image, 0);
139+
sysp_end = chrp_nvram_create_system_partition(image, 0, 0x1fd0);
140140

141141
/* Free space partition */
142142
chrp_nvram_create_free_partition(&image[sysp_end], 0x1fd0 - sysp_end);

include/hw/nvram/chrp_nvram.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ chrp_nvram_finish_partition(ChrpNvramPartHdr *header, uint32_t size)
5050
header->checksum = sum & 0xff;
5151
}
5252

53-
int chrp_nvram_create_system_partition(uint8_t *data, int min_len);
53+
/* chrp_nvram_create_system_partition() failure is fatal */
54+
int chrp_nvram_create_system_partition(uint8_t *data, int min_len, int max_len);
5455
int chrp_nvram_create_free_partition(uint8_t *data, int len);
5556

5657
#endif

0 commit comments

Comments
 (0)