Re: [PATCH] system/vl: Free allocate memory for pid file name in case realpath() failed

On 18/5/26 13:50, Thomas Huth wrote:
> On 16/05/2026 12.13, Philippe Mathieu-Daudé wrote:
>> On 15/5/26 16:38, Thomas Huth wrote:
>>> From: Thomas Huth <[email protected]>
>>>
>>> In case realpath() fails, the code returns early in the function
>>> qemu_maybe_daemonize(), without freeing the allocated memory. Add
>>> a g_free() here to fix it.
>>>
>>> Fixes: dee2a4d4d2f ("vl: defuse PID file path resolve error")
>>> Signed-off-by: Thomas Huth <[email protected]>
>>> ---
>>>   system/vl.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/system/vl.c b/system/vl.c
>>> index d2f4044e5d8..7e2ed898240 100644
>>> --- a/system/vl.c
>>> +++ b/system/vl.c
>>> @@ -2670,6 +2670,7 @@ static void qemu_maybe_daemonize(const char 
>>> *pid_file)
>>>                   warn_report("not removing PID file on exit: cannot 
>>> resolve PID "
>>>                               "file path: %s: %s", pid_file, 
>>> strerror(errno));
>>>               }
>>> +            g_free(pid_file_realpath);
>>>               return;
>>>           }
>>
>> Don't we also need to release the other paths?
>>
>>   - qemu_unlink_pidfile()
>>   - pid_file_init()
>>   - pid_file_cleanup()
> 
> Adding it to qemu_unlink_pidfile() makes sense here (though QEMU is 
> terminating anyway afterwards, some malloc sanitizers might complain 
> otherwise), so I sent a v2 for this.
> 
> pid_file_init() and pid_file_cleanup() is in the storage daemon, so this 
> is a separate problem (the variable just has the same name there). Not 
> sure whether it's worth the effort to free the memory there, too, since 
> the code exits afterwards anyway, and the pointer is kept 'til the end?

OK, fine!

> 
>   Thomas
> 
>