25.09.2023 22:40, Vladimir Sementsov-Ogievskiy wrote:
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
"And that makes sense". Just a nitpick in commit comment.
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;
Right now this is not important, since the only user of this
function, load_aout(), ignores errno value and reports general
failure instead. Original read_targphys() returned errno which
corresponds to failed read().
FWIW, at least load_aout() assumes we've read whole struct exec
from the file in question, which might not be the case.
/mjt
