[PATCH v2] qemu-pr-helper: do not free reply when multipath_process_cmd() fails

[PATCH v2] qemu-pr-helper: do not free reply when multipath_process_cmd() fails

[Stefan Hajnoczi]

Kevin Wolf pointed out that the multipath_process_cmd() documentation
only defines the value of reply on success. Don't touch it when the
function returns -1.

The libmpathcmd implementation as of c78cf64a9e8e does not set *reply to
NULL when its internal mpath_send_cmd() call fails, so I think Kevin's
interpretation is correct.

As it stands, there is no observable bug in the QEMU code though because
reply is initialized to NULL before calling multipath_process_cmd().
This patch is just a cleanup.

Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: kwolf@redhat.com
Cc: eblake@redhat.com
Cc: berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
v2:
- Use g_autofree instead of manual free(3) calls [Eric]
---
 scsi/qemu-pr-helper.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 54d196a503f..1f97a5c4bfe 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -559,7 +559,7 @@ static char *get_dm_name(int fd, Error **errp)
 static bool multipathd_cmd(int fd, const char *cmd, Error **errp)
 {
     const unsigned int timeout_ms = 60000;
-    char *reply = NULL;
+    g_autofree char *reply = NULL;
     int last_errno;
     int ret;
 
@@ -568,11 +568,16 @@ static bool multipathd_cmd(int fd, const char *cmd, Error **errp)
     if (reply && strcmp(reply, "ok\n") != 0) {
         error_setg(errp, "mpath_process_cmd(%d, \"%s\") replied \"%s\"",
                    fd, cmd, reply);
-        free(reply);
         return false;
     }
-    free(reply);
     if (ret == -1) {
+        /*
+         * It should already be NULL but the mpath_process_cmd() documentation
+         * only defines it when the function returns success, so don't depend
+         * on its value when ret == -1.
+         */
+        reply = NULL;
+
         error_setg_errno(errp, last_errno,
                          "mpath_process_cmd(%d, \"%s\") failed",
                          fd, cmd);
-- 
2.56.0

Re: [PATCH v2] qemu-pr-helper: do not free reply when multipath_process_cmd() fails

[Eric Blake]

On Tue, Oct 27, 2026 at 04:07:57PM -0400, Stefan Hajnoczi wrote:
> Kevin Wolf pointed out that the multipath_process_cmd() documentation
> only defines the value of reply on success. Don't touch it when the
> function returns -1.
> 
> The libmpathcmd implementation as of c78cf64a9e8e does not set *reply to
> NULL when its internal mpath_send_cmd() call fails, so I think Kevin's
> interpretation is correct.
> 
> As it stands, there is no observable bug in the QEMU code though because
> reply is initialized to NULL before calling multipath_process_cmd().
> This patch is just a cleanup.
> 
> Cc: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: kwolf@redhat.com
> Cc: eblake@redhat.com
> Cc: berrange@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> v2:
> - Use g_autofree instead of manual free(3) calls [Eric]

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org