Re: [PATCH-for-5.1 4/4] hw/avr/boot: Fix memory leak in avr_load_firmware()

[Philippe Mathieu-Daudé]

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
>