| Message ID | 20200714164257.23330-5-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | misc: Document qemu_find_file and fix memory leak in avr_load_firmware | expand |
ping? On 7/14/20 6:42 PM, Philippe Mathieu-Daudé wrote: > The value returned by qemu_find_file() must be freed. > > This fixes Coverity issue CID 1430449, which points out > that the memory returned by qemu_find_file() is leaked. > > Fixes: Coverity CID 1430449 (RESOURCE_LEAK) > Fixes: 7dd8f6fde4 ('hw/avr: Add support for loading ELF/raw binaries') > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/avr/boot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/avr/boot.c b/hw/avr/boot.c > index 6fbcde4061..151734f82d 100644 > --- a/hw/avr/boot.c > +++ b/hw/avr/boot.c > @@ -60,7 +60,7 @@ static const char *avr_elf_e_flags_to_cpu_type(uint32_t flags) > bool avr_load_firmware(AVRCPU *cpu, MachineState *ms, > MemoryRegion *program_mr, const char *firmware) > { > - const char *filename; > + g_autofree char *filename; > int bytes_loaded; > uint64_t entry; > uint32_t e_flags; >
On Tue, Jul 14, 2020 at 06:42:57PM +0200, Philippe Mathieu-Daudé wrote: > The value returned by qemu_find_file() must be freed. > > This fixes Coverity issue CID 1430449, which points out > that the memory returned by qemu_find_file() is leaked. > > Fixes: Coverity CID 1430449 (RESOURCE_LEAK) > Fixes: 7dd8f6fde4 ('hw/avr: Add support for loading ELF/raw binaries') > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/avr/boot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/avr/boot.c b/hw/avr/boot.c > index 6fbcde4061..151734f82d 100644 > --- a/hw/avr/boot.c > +++ b/hw/avr/boot.c > @@ -60,7 +60,7 @@ static const char *avr_elf_e_flags_to_cpu_type(uint32_t flags) > bool avr_load_firmware(AVRCPU *cpu, MachineState *ms, > MemoryRegion *program_mr, const char *firmware) > { > - const char *filename; > + g_autofree char *filename; Any variable marked g_autofree or g_auto must always be initialized to NULL otherwise there's risk of free'ing uninitialized data. Even if currently safe, any later refactoring could turn it into a bug. So iff "= NULL" is added: Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
On 7/20/20 4:18 PM, Daniel P. Berrangé wrote: > On Tue, Jul 14, 2020 at 06:42:57PM +0200, Philippe Mathieu-Daudé wrote: >> The value returned by qemu_find_file() must be freed. >> >> This fixes Coverity issue CID 1430449, which points out >> that the memory returned by qemu_find_file() is leaked. >> >> Fixes: Coverity CID 1430449 (RESOURCE_LEAK) >> Fixes: 7dd8f6fde4 ('hw/avr: Add support for loading ELF/raw binaries') >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/avr/boot.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/avr/boot.c b/hw/avr/boot.c >> index 6fbcde4061..151734f82d 100644 >> --- a/hw/avr/boot.c >> +++ b/hw/avr/boot.c >> @@ -60,7 +60,7 @@ static const char *avr_elf_e_flags_to_cpu_type(uint32_t flags) >> bool avr_load_firmware(AVRCPU *cpu, MachineState *ms, >> MemoryRegion *program_mr, const char *firmware) >> { >> - const char *filename; >> + g_autofree char *filename; > > Any variable marked g_autofree or g_auto must always be initialized > to NULL otherwise there's risk of free'ing uninitialized data. Even > if currently safe, any later refactoring could turn it into a bug. TIL, thanks :) > > So iff "= NULL" is added: > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > Regards, > Daniel >
diff --git a/hw/avr/boot.c b/hw/avr/boot.c index 6fbcde4061..151734f82d 100644 --- a/hw/avr/boot.c +++ b/hw/avr/boot.c @@ -60,7 +60,7 @@ static const char *avr_elf_e_flags_to_cpu_type(uint32_t flags) bool avr_load_firmware(AVRCPU *cpu, MachineState *ms, MemoryRegion *program_mr, const char *firmware) { - const char *filename; + g_autofree char *filename; int bytes_loaded; uint64_t entry; uint32_t e_flags;
The value returned by qemu_find_file() must be freed. This fixes Coverity issue CID 1430449, which points out that the memory returned by qemu_find_file() is leaked. Fixes: Coverity CID 1430449 (RESOURCE_LEAK) Fixes: 7dd8f6fde4 ('hw/avr: Add support for loading ELF/raw binaries') Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/avr/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)