Reviewed-by: Konstantin Kostiuk <kkost...@redhat.com>
On Mon, Sep 18, 2023 at 2:00 PM Daniel Xu <d...@dxuuu.xyz> wrote:
> If capture-output is requested but one of the channels goes unused (eg.
> we attempt to capture stderr but the command never writes to stderr), we
> can leak memory.
>
> guest_exec_output_watch() is (from what I understand) unconditionally
> called for both streams if output capture is requested. The first call
> will always pass the `p->size == p->length` check b/c both values are
> 0. Then GUEST_EXEC_IO_SIZE bytes will be allocated for the stream.
>
> But when we reap the exited process there's a `gei->err.length > 0`
> check to actually free the buffer. Which does not get run if the command
> doesn't write to the stream.
>
> Fix by making free() unconditional.
>
> Signed-off-by: Daniel Xu <d...@dxuuu.xyz>
> ---
> qga/commands.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands.c b/qga/commands.c
> index 09c683e263..ce172edd2d 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -206,15 +206,15 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid,
> Error **errp)
> #endif
> if (gei->out.length > 0) {
> ges->out_data = g_base64_encode(gei->out.data,
> gei->out.length);
> - g_free(gei->out.data);
> ges->has_out_truncated = gei->out.truncated;
> }
> + g_free(gei->out.data);
>
> if (gei->err.length > 0) {
> ges->err_data = g_base64_encode(gei->err.data,
> gei->err.length);
> - g_free(gei->err.data);
> ges->has_err_truncated = gei->err.truncated;
> }
> + g_free(gei->err.data);
>
> QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
> g_free(gei);
> --
> 2.41.0
>
>
