Commit d09acb9b authored by Markus Armbruster's avatar Markus Armbruster Committed by Blue Swirl
Browse files

fw_cfg: Splash image loader can overrun a stack variable, fix 



read_splashfile() passes the address of an int variable as size_t *
parameter to g_file_get_contents(), with a cast to gag the compiler.

No problem on machines where sizeof(size_t) == sizeof(int).

Happens to work on my x86_64 box (64 bit little endian): the least
significant 32 bits of the file size end up in the right place
(caller's variable file_size), and the most significant 32 bits
clobber a place that gets assigned to before its next use (caller's
variable file_type).

I'd expect it to break on a 64 bit big-endian box.

Fix up the variable types and drop the problematic cast.

Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
Signed-off-by: default avatarBlue Swirl <blauwirbel@gmail.com>
parent a6e7c184

hw/fw_cfg.c

+4 −3
Original line number Diff line number Diff line
@@ -54,7 +54,8 @@ struct FWCfgState { 
#define JPG_FILE 0

#define BMP_FILE 1



static char *read_splashfile(char *filename, int *file_sizep, int *file_typep)

static char *read_splashfile(char *filename, size_t *file_sizep,

                             int *file_typep)

{

    GError *err = NULL;

    gboolean res;
@@ -63,7 +64,7 @@ static char *read_splashfile(char *filename, int *file_sizep, int *file_typep) 
    unsigned int filehead = 0;

    int bmp_bpp;



    res = g_file_get_contents(filename, &content, (gsize *)file_sizep, &err);

    res = g_file_get_contents(filename, &content, file_sizep, &err);

    if (res == FALSE) {

        error_report("failed to read splash file '%s'", filename);

        g_error_free(err);
@@ -111,7 +112,7 @@ static void fw_cfg_bootsplash(FWCfgState *s) 
    const char *boot_splash_filename = NULL;

    char *p;

    char *filename, *file_data;

    int file_size;

    size_t file_size;

    int file_type = -1;

    const char *temp;

include/sysemu/sysemu.h

+1 −1
Original line number Diff line number Diff line
@@ -122,7 +122,7 @@ extern int semihosting_enabled; 
extern int old_param;

extern int boot_menu;

extern uint8_t *boot_splash_filedata;

extern int boot_splash_filedata_size;

extern size_t boot_splash_filedata_size;

extern uint8_t qemu_extra_params_fw[2];

extern QEMUClock *rtc_clock;

vl.c

+1 −1
Original line number Diff line number Diff line
@@ -231,7 +231,7 @@ unsigned int nb_prom_envs = 0; 
const char *prom_envs[MAX_PROM_ENVS];

int boot_menu;

uint8_t *boot_splash_filedata;

int boot_splash_filedata_size;

size_t boot_splash_filedata_size;

uint8_t qemu_extra_params_fw[2];



typedef struct FWBootEntry FWBootEntry;