[Qemu-devel] [PULL 0/4] xen queue 2019-02-28

[Qemu-devel] [PULL 0/4] xen queue 2019-02-28

[Anthony PERARD]

The following changes since commit 711d13d5e2e160c1c3bcbd302af6df3980a99469:

  Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-feb-27-2019' into staging (2019-02-28 12:59:49 +0000)

are available in the Git repository at:

  https://xenbits.xen.org/git-http/people/aperard/qemu-dm.git tags/pull-xen-20190228

for you to fetch changes up to 156ac94463b42b0b9beea263af9866dfcd3683e0:

  xen-block: stop leaking memory in xen_block_drive_create() (2019-02-28 17:21:12 +0000)

----------------------------------------------------------------
Xen queue

* xen-block fixes

----------------------------------------------------------------
Paul Durrant (4):
      dataplane/xen-block: remove dead code
      xen-block: remove redundant assignment
      xen-block: report error condition from vbd_name_to_disk()
      xen-block: stop leaking memory in xen_block_drive_create()

 hw/block/dataplane/xen-block.c |  5 +----
 hw/block/xen-block.c           | 40 +++++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 19 deletions(-)

[Qemu-devel] [PULL 1/4] dataplane/xen-block: remove dead code

[Anthony PERARD]

From: Paul Durrant <paul.durrant@citrix.com>

The if() statement is clearly bogus (dead code which should have been
cleaned up when grant mapping was removed).

Spotted by Coverity: CID 1398635

While in the neighbourhood, add a missing 'fall through' annotation.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Message-Id: <20190215162533.19475-2-paul.durrant@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/block/dataplane/xen-block.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index c6a15da024..f1523c5b45 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -281,10 +281,6 @@ static void xen_block_complete_aio(void *opaque, int ret)
         break;
     case BLKIF_OP_WRITE:
     case BLKIF_OP_FLUSH_DISKCACHE:
-        if (!request->req.nr_segments) {
-            break;
-        }
-        break;
     default:
         break;
     }
@@ -298,6 +294,7 @@ static void xen_block_complete_aio(void *opaque, int ret)
         if (!request->req.nr_segments) {
             break;
         }
+        /* fall through */
     case BLKIF_OP_READ:
         if (request->status == BLKIF_RSP_OKAY) {
             block_acct_done(blk_get_stats(dataplane->blk), &request->acct);
-- 
Anthony PERARD

[Qemu-devel] [PULL 2/4] xen-block: remove redundant assignment

[Anthony PERARD]

From: Paul Durrant <paul.durrant@citrix.com>

The assignment to 'p' is unnecessary as the code will either goto 'invalid'
or p will get overwritten.

Spotted by Coverity: CID 1398638

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Message-Id: <20190215162533.19475-3-paul.durrant@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/block/xen-block.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 5012af9cb6..29afe2703a 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -413,8 +413,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
         }
 
         if (*end == 'p') {
-            p = (char *) ++end;
-            if (*end == '\0') {
+            if (*(++end) == '\0') {
                 goto invalid;
             }
         }
-- 
Anthony PERARD

[Qemu-devel] [PULL 3/4] xen-block: report error condition from vbd_name_to_disk()

[Anthony PERARD]

From: Paul Durrant <paul.durrant@citrix.com>

The function needs to make sure it is passed a valid disk name. This is
easily done by making sure that the parsing loop results in a non-zero
value.

Spotted by Coverity: CID 1398640

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Message-Id: <20190215162533.19475-4-paul.durrant@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/block/xen-block.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 29afe2703a..37a456c207 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -351,21 +351,28 @@ static void xen_block_get_vdev(Object *obj, Visitor *v, const char *name,
     g_free(str);
 }
 
-static unsigned int vbd_name_to_disk(const char *name, const char **endp)
+static int vbd_name_to_disk(const char *name, const char **endp,
+                            unsigned long *disk)
 {
-    unsigned int disk = 0;
+    unsigned int n = 0;
 
     while (*name != '\0') {
         if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) {
             break;
         }
 
-        disk *= 26;
-        disk += *name++ - 'a' + 1;
+        n *= 26;
+        n += *name++ - 'a' + 1;
     }
     *endp = name;
 
-    return disk - 1;
+    if (!n) {
+        return -1;
+    }
+
+    *disk = n - 1;
+
+    return 0;
 }
 
 static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
@@ -418,7 +425,9 @@ static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
             }
         }
     } else {
-        vdev->disk = vbd_name_to_disk(p, &end);
+        if (vbd_name_to_disk(p, &end, &vdev->disk)) {
+            goto invalid;
+        }
     }
 
     if (*end != '\0') {
-- 
Anthony PERARD

[Qemu-devel] [PULL 4/4] xen-block: stop leaking memory in xen_block_drive_create()

[Anthony PERARD]

From: Paul Durrant <paul.durrant@citrix.com>

The locally allocated QDict-s need to be freed. ('file_layer' will be
freed implicitly since it is added as an object to 'driver_layer').

Spotted by Coverity: CID 1398649

While in the neighbourhood free 'driver' and 'filename' as soon as they are
added to the QDicts. Freeing after the 'done' label doesn't make that much
sense as, if the error path jumps to that label, the values would be NULL
anyway.

This patch also makes that more obvious by taking the error path if
'params' is NULL and then asserting that both driver and filename are
non-NULL in the normal path.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Message-Id: <20190219163440.15702-1-paul.durrant@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/block/xen-block.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 37a456c207..70fc2455e8 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -743,12 +743,12 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
         }
 
         g_strfreev(v);
-    }
-
-    if (!filename) {
-        error_setg(errp, "no filename");
+    } else {
+        error_setg(errp, "no params");
         goto done;
     }
+
+    assert(filename);
     assert(driver);
 
     drive = g_new0(XenBlockDrive, 1);
@@ -758,6 +758,7 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
 
     qdict_put_str(file_layer, "driver", "file");
     qdict_put_str(file_layer, "filename", filename);
+    g_free(filename);
 
     if (mode && *mode != 'w') {
         qdict_put_bool(file_layer, "read-only", true);
@@ -793,16 +794,17 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
     driver_layer = qdict_new();
 
     qdict_put_str(driver_layer, "driver", driver);
+    g_free(driver);
+
     qdict_put_obj(driver_layer, "file", QOBJECT(file_layer));
 
     g_assert(!drive->node_name);
     drive->node_name = xen_block_blockdev_add(drive->id, driver_layer,
                                               &local_err);
 
-done:
-    g_free(driver);
-    g_free(filename);
+    qobject_unref(driver_layer);
 
+done:
     if (local_err) {
         error_propagate(errp, local_err);
         xen_block_drive_destroy(drive, NULL);
-- 
Anthony PERARD

Re: [Qemu-devel] [PULL 0/4] xen queue 2019-02-28

[Peter Maydell]

On Thu, 28 Feb 2019 at 17:34, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> The following changes since commit 711d13d5e2e160c1c3bcbd302af6df3980a99469:
>
>   Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-feb-27-2019' into staging (2019-02-28 12:59:49 +0000)
>
> are available in the Git repository at:
>
>   https://xenbits.xen.org/git-http/people/aperard/qemu-dm.git tags/pull-xen-20190228
>
> for you to fetch changes up to 156ac94463b42b0b9beea263af9866dfcd3683e0:
>
>   xen-block: stop leaking memory in xen_block_drive_create() (2019-02-28 17:21:12 +0000)
>
> ----------------------------------------------------------------
> Xen queue
>
> * xen-block fixes
>
> ----------------------------------------------------------------
> Paul Durrant (4):
>       dataplane/xen-block: remove dead code
>       xen-block: remove redundant assignment
>       xen-block: report error condition from vbd_name_to_disk()
>       xen-block: stop leaking memory in xen_block_drive_create()

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM