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

Thomas Huth <[email protected]> Mon, 18 May 2026 13:50:06 +0200
Newsgroups org.nongnu.qemu-trivial,org.nongnu.qemu-devel
Message-ID <[email protected]>
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?

  Thomas