[PATCH] qga: Fix crash due to redundant error setting in qmp_guest_file_open()

[PATCH] qga: Fix crash due to redundant error setting in qmp_guest_file_open()

[Wei Wang]

Avoid setting GError multiple times in qmp_guest_file_open() by checking
if errp is already set.
This prevents crash caused by repeated error handling calls.

Signed-off-by: Wei Wang <wei.wang@smartx.com>
---
 qga/commands-win32.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0fd0c966e4..8c45ca5004 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -263,7 +263,9 @@ int64_t qmp_guest_file_open(const char *path, const char *mode, Error **errp)
     fd = guest_file_handle_add(fh, errp);
     if (fd < 0) {
         CloseHandle(fh);
-        error_setg(errp, "failed to add handle to qmp handle table");
+        if (!*errp) {
+            error_setg(errp, "failed to add handle to qmp handle table");
+        }
         goto done;
     }
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] qga: Fix crash due to redundant error setting in qmp_guest_file_open()

[Daniel P. Berrangé]

On Thu, Jan 22, 2026 at 12:04:30PM +0800, Wei Wang wrote:
> Avoid setting GError multiple times in qmp_guest_file_open() by checking
> if errp is already set.
> This prevents crash caused by repeated error handling calls.
> 
> Signed-off-by: Wei Wang <wei.wang@smartx.com>
> ---
>  qga/commands-win32.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 0fd0c966e4..8c45ca5004 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -263,7 +263,9 @@ int64_t qmp_guest_file_open(const char *path, const char *mode, Error **errp)
>      fd = guest_file_handle_add(fh, errp);
>      if (fd < 0) {
>          CloseHandle(fh);
> -        error_setg(errp, "failed to add handle to qmp handle table");
> +        if (!*errp) {
> +            error_setg(errp, "failed to add handle to qmp handle table");
> +        }

guest_file_handle_add() only returns -1 when qga_get_fd_handle
returns -1, and qga_get_fd_handle always sets errp.

Therefore, your new conditional call to error_setg will never be
executed; remove it entirely.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] qga: Fix crash due to redundant error setting in qmp_guest_file_open()

[Markus Armbruster]

Wei Wang <wei.wang@smartx.com> writes:

> Avoid setting GError multiple times in qmp_guest_file_open() by checking
> if errp is already set.
> This prevents crash caused by repeated error handling calls.
>
> Signed-off-by: Wei Wang <wei.wang@smartx.com>
> ---
>  qga/commands-win32.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 0fd0c966e4..8c45ca5004 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -263,7 +263,9 @@ int64_t qmp_guest_file_open(const char *path, const char *mode, Error **errp)
>      fd = guest_file_handle_add(fh, errp);
>      if (fd < 0) {
>          CloseHandle(fh);
> -        error_setg(errp, "failed to add handle to qmp handle table");
> +        if (!*errp) {
> +            error_setg(errp, "failed to add handle to qmp handle table");
> +        }
>          goto done;
>      }

Can guest_file_handle_add() fail without setting an error?

If yes, that's the bug that needs fixing.

If no, the error_setg() here is wrong and needs to be deleted.

Re: [PATCH] qga: Fix crash due to redundant error setting in qmp_guest_file_open()

[Kostiantyn Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Thu, Jan 22, 2026 at 10:57 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Wei Wang <wei.wang@smartx.com> writes:
>
> > Avoid setting GError multiple times in qmp_guest_file_open() by checking
> > if errp is already set.
> > This prevents crash caused by repeated error handling calls.
> >
> > Signed-off-by: Wei Wang <wei.wang@smartx.com>
> > ---
> >  qga/commands-win32.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 0fd0c966e4..8c45ca5004 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -263,7 +263,9 @@ int64_t qmp_guest_file_open(const char *path, const
> char *mode, Error **errp)
> >      fd = guest_file_handle_add(fh, errp);
> >      if (fd < 0) {
> >          CloseHandle(fh);
> > -        error_setg(errp, "failed to add handle to qmp handle table");
> > +        if (!*errp) {
> > +            error_setg(errp, "failed to add handle to qmp handle
> table");
> > +        }
> >          goto done;
> >      }
>
> Can guest_file_handle_add() fail without setting an error?
>

I checked the code, and it looks like guest_file_handle_add always
set error if fail


>
> If yes, that's the bug that needs fixing.
>
> If no, the error_setg() here is wrong and needs to be deleted.
>
>

[-- Attachment #2: Type: text/html, Size: 2168 bytes --]