On Mon, 25 Sept 2023 at 20:41, Vladimir Sementsov-Ogievskiy
<vsement...@yandex-team.ru> wrote:
>
> This @size parameter often comes from fd. We'd better check it before
> doing read and allocation.
>
> Chose 1G as high enough empiric bound.
Empirical for who?
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
> ---
> hw/core/loader.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 4dd5a71fb7..4b67543046 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -281,11 +281,26 @@ ssize_t load_aout(const char *filename, hwaddr addr,
> int max_sz,
>
> /* ELF loader */
>
> +#define ELF_LOAD_MAX (1024 * 1024 * 1024)
> +
> static void *load_at(int fd, off_t offset, size_t size)
> {
> void *ptr;
> - if (lseek(fd, offset, SEEK_SET) < 0)
> +
> + /*
> + * We often come here with @size, which was previously read from file
> + * descriptor too. That's not good to read and allocate for unchecked
> + * number of bytes. Coverity also doesn't like it and generate problems.
> + * So, let's limit all load_at() calls to ELF_LOAD_MAX at least.
> + */
> + if (size > ELF_LOAD_MAX) {
> return NULL;
> + }
> +
> + if (lseek(fd, offset, SEEK_SET) < 0) {
> + return NULL;
> + }
> +
> ptr = g_malloc(size);
> if (read(fd, ptr, size) != size) {
> g_free(ptr);
This doesn't really help anything:
(1) if the value is really big, it doesn't cause any terrible
consequences -- QEMU will just exit because the allocation
fails, which is fine because this will be at QEMU startup
and only happens if the user running QEMU gives us a silly file
(2) we do a lot of other "allocate and abort on failure"
elsewhere in the ELF loader, for instance the allocations of
the symbol table and relocs in the load_symbols and
elf_reloc functions, and then on a bigger scale when we
work with the actual data in the ELF file
thanks
-- PMM
