On Tue, 26 Sept 2023 at 11:51, Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote: > > On 26.09.23 13:33, Peter Maydell wrote: > > 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 > > Reasonable.. > > Don't you have an idea, how to somehow mark the value "trusted" for Coverity?
In the web UI, I just mark it "false positive" in the dropdown, and move on. Coverity has an absolute ton of false positives, and you really can't work with it unless you have a workflow for ignoring them. thanks -- PMM