Coverity doesn't like using "untrusted" values, coming from buffers and
fd-s as length to do IO and allocations. And that's make sense. The
function is used three times with "untrusted" nbytes parameter. Let's
introduce at least empirical limit of 1G for it.
While being here make the function static, as it's used only here.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
hw/core/loader.c | 13 ++++++++++---
include/hw/loader.h | 2 --
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa02b27089..48cff6f59e 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -101,17 +101,24 @@ ssize_t load_image_size(const char *filename, void *addr,
size_t size)
return actsize < 0 ? -1 : l;
}
+#define READ_TARGPHYS_MAX_BYTES (1024 * 1024 * 1024)
/* read()-like version */
-ssize_t read_targphys(const char *name,
- int fd, hwaddr dst_addr, size_t nbytes)
+static ssize_t read_targphys(const char *name,
+ int fd, hwaddr dst_addr, size_t nbytes)
{
uint8_t *buf;
ssize_t did;
+ if (nbytes > READ_TARGPHYS_MAX_BYTES) {
+ return -1;
+ }
+
buf = g_malloc(nbytes);
did = read(fd, buf, nbytes);
- if (did > 0)
+ if (did > 0) {
rom_add_blob_fixed("read", buf, did, dst_addr);
+ }
+
g_free(buf);
return did;
}
diff --git a/include/hw/loader.h b/include/hw/loader.h
index c4c14170ea..e29af233d2 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -264,8 +264,6 @@ ssize_t load_ramdisk(const char *filename, hwaddr addr,
uint64_t max_sz);
ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src, size_t srclen);
-ssize_t read_targphys(const char *name,
- int fd, hwaddr dst_addr, size_t nbytes);
void pstrcpy_targphys(const char *name,
hwaddr dest, int buf_size,
const char *source);
--
2.34.1
