hw/core/eif.c | 48 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 7 deletions(-)
Check for overflow as well as allocation failure. Resolves Coverity CID 1564859.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/eif.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/hw/core/eif.c b/hw/core/eif.c
index cbcd80de58b..25f2aedf3fa 100644
--- a/hw/core/eif.c
+++ b/hw/core/eif.c
@@ -123,6 +123,10 @@ static bool read_eif_header(FILE *f, EifHeader *header, uint32_t *crc,
for (int i = 0; i < MAX_SECTIONS; ++i) {
header->section_sizes[i] = be64_to_cpu(header->section_sizes[i]);
+ if (header->section_sizes[i] > SSIZE_MAX) {
+ error_setg(errp, "Invalid EIF image. Section size out of bounds");
+ return false;
+ }
}
header->unused = be32_to_cpu(header->unused);
@@ -282,7 +286,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
struct cbor_load_result result;
bool ret = false;
- sig = g_malloc(size);
+ sig = g_try_malloc(size);
+ if (!sig) {
+ error_setg(errp, "Out of memory reading signature section");
+ goto cleanup;
+ }
+
got = fread(sig, 1, size, eif);
if ((uint64_t) got != size) {
error_setg(errp, "Failed to read EIF signature section data");
@@ -324,7 +333,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
error_setg(errp, "Invalid signature CBOR");
goto cleanup;
}
- cert = g_malloc(len);
+ cert = g_try_malloc(len);
+ if (!cert) {
+ error_setg(errp, "Out of memory reading signature section");
+ goto cleanup;
+ }
+
for (int i = 0; i < len; ++i) {
cbor_item_t *tmp = cbor_array_get(pair->value, i);
if (!tmp) {
@@ -503,7 +517,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
goto cleanup;
}
- ptr = g_malloc(hdr.section_size);
+ ptr = g_try_malloc(hdr.section_size);
+ if (!ptr) {
+ error_setg(errp, "Out of memory reading kernel section");
+ goto cleanup;
+ }
iov_ptr = g_malloc(sizeof(struct iovec));
iov_ptr->iov_base = ptr;
@@ -528,7 +546,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
goto cleanup;
}
size = hdr.section_size;
- *cmdline = g_malloc(size + 1);
+ *cmdline = g_try_malloc(size + 1);
+ if (!cmdline) {
+ error_setg(errp, "Out of memory reading command line section");
+ goto cleanup;
+ }
if (!read_eif_cmdline(f, size, *cmdline, &crc, errp)) {
goto cleanup;
}
@@ -567,7 +589,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
}
}
- ptr = g_malloc(hdr.section_size);
+ ptr = g_try_malloc(hdr.section_size);
+ if (!ptr) {
+ error_setg(errp, "Out of memory reading initrd section");
+ goto cleanup;
+ }
iov_ptr = g_malloc(sizeof(struct iovec));
iov_ptr->iov_base = ptr;
@@ -606,7 +632,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
uint8_t *buf;
size_t got;
uint64_t size = hdr.section_size;
- buf = g_malloc(size);
+ buf = g_try_malloc(size);
+ if (!buf) {
+ error_setg(errp, "Out of memory reading unknown section");
+ goto cleanup;
+ }
got = fread(buf, 1, size, f);
if ((uint64_t) got != size) {
g_free(buf);
@@ -662,7 +692,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
goto cleanup;
}
- ptr = g_malloc(machine_initrd_size);
+ ptr = g_try_malloc(machine_initrd_size);
+ if (!ptr) {
+ error_setg(errp, "Out of memory reading initrd file");
+ goto cleanup;
+ }
iov_ptr = g_malloc(sizeof(struct iovec));
iov_ptr->iov_base = ptr;
--
2.47.0On Wed, Nov 6, 2024 at 11:44 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Check for overflow as well as allocation failure. Resolves Coverity CID 1564859.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/core/eif.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/eif.c b/hw/core/eif.c
> index cbcd80de58b..25f2aedf3fa 100644
> --- a/hw/core/eif.c
> +++ b/hw/core/eif.c
> @@ -123,6 +123,10 @@ static bool read_eif_header(FILE *f, EifHeader *header, uint32_t *crc,
>
> for (int i = 0; i < MAX_SECTIONS; ++i) {
> header->section_sizes[i] = be64_to_cpu(header->section_sizes[i]);
> + if (header->section_sizes[i] > SSIZE_MAX) {
> + error_setg(errp, "Invalid EIF image. Section size out of bounds");
> + return false;
> + }
> }
>
> header->unused = be32_to_cpu(header->unused);
> @@ -282,7 +286,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
> struct cbor_load_result result;
> bool ret = false;
>
> - sig = g_malloc(size);
> + sig = g_try_malloc(size);
> + if (!sig) {
> + error_setg(errp, "Out of memory reading signature section");
> + goto cleanup;
> + }
> +
> got = fread(sig, 1, size, eif);
> if ((uint64_t) got != size) {
> error_setg(errp, "Failed to read EIF signature section data");
> @@ -324,7 +333,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
> error_setg(errp, "Invalid signature CBOR");
> goto cleanup;
> }
> - cert = g_malloc(len);
> + cert = g_try_malloc(len);
> + if (!cert) {
> + error_setg(errp, "Out of memory reading signature section");
> + goto cleanup;
> + }
> +
> for (int i = 0; i < len; ++i) {
> cbor_item_t *tmp = cbor_array_get(pair->value, i);
> if (!tmp) {
> @@ -503,7 +517,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> goto cleanup;
> }
>
> - ptr = g_malloc(hdr.section_size);
> + ptr = g_try_malloc(hdr.section_size);
> + if (!ptr) {
> + error_setg(errp, "Out of memory reading kernel section");
> + goto cleanup;
> + }
>
> iov_ptr = g_malloc(sizeof(struct iovec));
> iov_ptr->iov_base = ptr;
> @@ -528,7 +546,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> goto cleanup;
> }
> size = hdr.section_size;
> - *cmdline = g_malloc(size + 1);
> + *cmdline = g_try_malloc(size + 1);
> + if (!cmdline) {
> + error_setg(errp, "Out of memory reading command line section");
> + goto cleanup;
> + }
I was looking into doing some changes on top of this patch and this
check above should be if (!(*cmdline)), right?
Regards,
Dorjoy
On Wed, Nov 6, 2024 at 11:44 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > Check for overflow as well as allocation failure. Resolves Coverity CID 1564859. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/core/eif.c | 48 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 41 insertions(+), 7 deletions(-) > Reviewed-by: Dorjoy Chowdhury <dorjoychy111@gmail.com> Thanks for fixing! Regards, Dorjoy
On 11/6/24 09:44, Paolo Bonzini wrote:
> Check for overflow as well as allocation failure. Resolves Coverity CID 1564859.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/core/eif.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/eif.c b/hw/core/eif.c
> index cbcd80de58b..25f2aedf3fa 100644
> --- a/hw/core/eif.c
> +++ b/hw/core/eif.c
> @@ -123,6 +123,10 @@ static bool read_eif_header(FILE *f, EifHeader *header, uint32_t *crc,
>
> for (int i = 0; i < MAX_SECTIONS; ++i) {
> header->section_sizes[i] = be64_to_cpu(header->section_sizes[i]);
> + if (header->section_sizes[i] > SSIZE_MAX) {
> + error_setg(errp, "Invalid EIF image. Section size out of bounds");
> + return false;
> + }
> }
>
> header->unused = be32_to_cpu(header->unused);
> @@ -282,7 +286,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
> struct cbor_load_result result;
> bool ret = false;
>
> - sig = g_malloc(size);
> + sig = g_try_malloc(size);
> + if (!sig) {
> + error_setg(errp, "Out of memory reading signature section");
> + goto cleanup;
> + }
> +
> got = fread(sig, 1, size, eif);
> if ((uint64_t) got != size) {
> error_setg(errp, "Failed to read EIF signature section data");
> @@ -324,7 +333,12 @@ static bool get_signature_fingerprint_sha384(FILE *eif, uint64_t size,
> error_setg(errp, "Invalid signature CBOR");
> goto cleanup;
> }
> - cert = g_malloc(len);
> + cert = g_try_malloc(len);
> + if (!cert) {
> + error_setg(errp, "Out of memory reading signature section");
> + goto cleanup;
> + }
> +
> for (int i = 0; i < len; ++i) {
> cbor_item_t *tmp = cbor_array_get(pair->value, i);
> if (!tmp) {
> @@ -503,7 +517,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> goto cleanup;
> }
>
> - ptr = g_malloc(hdr.section_size);
> + ptr = g_try_malloc(hdr.section_size);
> + if (!ptr) {
> + error_setg(errp, "Out of memory reading kernel section");
> + goto cleanup;
> + }
>
> iov_ptr = g_malloc(sizeof(struct iovec));
> iov_ptr->iov_base = ptr;
> @@ -528,7 +546,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> goto cleanup;
> }
> size = hdr.section_size;
> - *cmdline = g_malloc(size + 1);
> + *cmdline = g_try_malloc(size + 1);
> + if (!cmdline) {
> + error_setg(errp, "Out of memory reading command line section");
> + goto cleanup;
> + }
> if (!read_eif_cmdline(f, size, *cmdline, &crc, errp)) {
> goto cleanup;
> }
> @@ -567,7 +589,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> }
> }
>
> - ptr = g_malloc(hdr.section_size);
> + ptr = g_try_malloc(hdr.section_size);
> + if (!ptr) {
> + error_setg(errp, "Out of memory reading initrd section");
> + goto cleanup;
> + }
>
> iov_ptr = g_malloc(sizeof(struct iovec));
> iov_ptr->iov_base = ptr;
> @@ -606,7 +632,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> uint8_t *buf;
> size_t got;
> uint64_t size = hdr.section_size;
> - buf = g_malloc(size);
> + buf = g_try_malloc(size);
> + if (!buf) {
> + error_setg(errp, "Out of memory reading unknown section");
> + goto cleanup;
> + }
> got = fread(buf, 1, size, f);
> if ((uint64_t) got != size) {
> g_free(buf);
> @@ -662,7 +692,11 @@ bool read_eif_file(const char *eif_path, const char *machine_initrd,
> goto cleanup;
> }
>
> - ptr = g_malloc(machine_initrd_size);
> + ptr = g_try_malloc(machine_initrd_size);
> + if (!ptr) {
> + error_setg(errp, "Out of memory reading initrd file");
> + goto cleanup;
> + }
>
> iov_ptr = g_malloc(sizeof(struct iovec));
> iov_ptr->iov_base = ptr;
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
© 2016 - 2024 Red Hat, Inc.