[Qemu-devel] [PATCH] loader: Fix read_targphys() to behave when read() fails

[Qemu-devel] [PATCH] loader: Fix read_targphys() to behave when read() fails

[Markus Armbruster]

Happily passes (size_t)-1 to rom_add_blob_fixed(), which promptly dies
attempting to malloc that much.  Spotted by Coverity.

Bonus fix for ROMs larger than INT_MAX bytes: return ssize_t instead
of int.  Bug can't bite, because the only user load_aout() limits ROM
size to an int value.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/loader.c |    9 +++++----
 hw/loader.h |    4 ++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/loader.c b/hw/loader.c
index 5676c18..9bbcddd 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -85,11 +85,11 @@ int load_image(const char *filename, uint8_t *addr)
 }
 
 /* read()-like version */
-int read_targphys(const char *name,
-                  int fd, target_phys_addr_t dst_addr, size_t nbytes)
+ssize_t read_targphys(const char *name,
+                      int fd, target_phys_addr_t dst_addr, size_t nbytes)
 {
     uint8_t *buf;
-    size_t did;
+    ssize_t did;
 
     buf = g_malloc(nbytes);
     did = read(fd, buf, nbytes);
@@ -176,7 +176,8 @@ static void bswap_ahdr(struct exec *e)
 int load_aout(const char *filename, target_phys_addr_t addr, int max_sz,
               int bswap_needed, target_phys_addr_t target_page_size)
 {
-    int fd, size, ret;
+    int fd;
+    ssize_t size, ret;
     struct exec e;
     uint32_t magic;
 
diff --git a/hw/loader.h b/hw/loader.h
index fc6bdff..fbcaba9 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -14,8 +14,8 @@ int load_aout(const char *filename, target_phys_addr_t addr, int max_sz,
 int load_uimage(const char *filename, target_phys_addr_t *ep,
                 target_phys_addr_t *loadaddr, int *is_linux);
 
-int read_targphys(const char *name,
-                  int fd, target_phys_addr_t dst_addr, size_t nbytes);
+ssize_t read_targphys(const char *name,
+                      int fd, target_phys_addr_t dst_addr, size_t nbytes);
 void pstrcpy_targphys(const char *name,
                       target_phys_addr_t dest, int buf_size,
                       const char *source);
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH] loader: Fix read_targphys() to behave when read() fails

[Blue Swirl]

Thanks, applied.

On Wed, Nov 16, 2011 at 18:41, Markus Armbruster <armbru@redhat.com> wrote:
> Happily passes (size_t)-1 to rom_add_blob_fixed(), which promptly dies
> attempting to malloc that much.  Spotted by Coverity.
>
> Bonus fix for ROMs larger than INT_MAX bytes: return ssize_t instead
> of int.  Bug can't bite, because the only user load_aout() limits ROM
> size to an int value.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/loader.c |    9 +++++----
>  hw/loader.h |    4 ++--
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 5676c18..9bbcddd 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -85,11 +85,11 @@ int load_image(const char *filename, uint8_t *addr)
>  }
>
>  /* read()-like version */
> -int read_targphys(const char *name,
> -                  int fd, target_phys_addr_t dst_addr, size_t nbytes)
> +ssize_t read_targphys(const char *name,
> +                      int fd, target_phys_addr_t dst_addr, size_t nbytes)
>  {
>     uint8_t *buf;
> -    size_t did;
> +    ssize_t did;
>
>     buf = g_malloc(nbytes);
>     did = read(fd, buf, nbytes);
> @@ -176,7 +176,8 @@ static void bswap_ahdr(struct exec *e)
>  int load_aout(const char *filename, target_phys_addr_t addr, int max_sz,
>               int bswap_needed, target_phys_addr_t target_page_size)
>  {
> -    int fd, size, ret;
> +    int fd;
> +    ssize_t size, ret;
>     struct exec e;
>     uint32_t magic;
>
> diff --git a/hw/loader.h b/hw/loader.h
> index fc6bdff..fbcaba9 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -14,8 +14,8 @@ int load_aout(const char *filename, target_phys_addr_t addr, int max_sz,
>  int load_uimage(const char *filename, target_phys_addr_t *ep,
>                 target_phys_addr_t *loadaddr, int *is_linux);
>
> -int read_targphys(const char *name,
> -                  int fd, target_phys_addr_t dst_addr, size_t nbytes);
> +ssize_t read_targphys(const char *name,
> +                      int fd, target_phys_addr_t dst_addr, size_t nbytes);
>  void pstrcpy_targphys(const char *name,
>                       target_phys_addr_t dest, int buf_size,
>                       const char *source);
> --
> 1.7.6.4
>
>