qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-2.10] block/nfs: fix mutex assertion in nfs_


From: Peter Lieven
Subject: Re: [Qemu-block] [PATCH for-2.10] block/nfs: fix mutex assertion in nfs_file_close()
Date: Tue, 8 Aug 2017 13:06:17 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

Am 08.08.2017 um 00:29 schrieb Jeff Cody:
Commit c096358e747e88fc7364e40e3c354ee0bb683960 introduced assertion
checks for when qemu_mutex() functions are called without the
corresponding qemu_mutex_init() having initialized the mutex.

This uncovered a latent bug in qemu's nfs driver - in
nfs_client_close(), the NFSClient structure is overwritten with zeros,
prior to the mutex being destroyed.

Go ahead and destroy the mutex in nfs_client_close(), and change where
we call qemu_mutex_init() so that it is correctly balanced.

There are also a couple of memory leaks obscured by the memset, so this
fixes those as well.

Finally, we should be able to get rid of the memset(), as it isn't
necessary.

Signed-off-by: Jeff Cody <address@hidden>
---
  block/nfs.c | 21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index d8db419..bec16b7 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -433,19 +433,23 @@ static void nfs_client_close(NFSClient *client)
      if (client->context) {
          if (client->fh) {
              nfs_close(client->context, client->fh);
+            client->fh = NULL;
          }
          aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
                             false, NULL, NULL, NULL, NULL);
          nfs_destroy_context(client->context);
+        client->context = NULL;
      }
-    memset(client, 0, sizeof(NFSClient));
-}
-
-static void nfs_file_close(BlockDriverState *bs)
-{
-    NFSClient *client = bs->opaque;
-    nfs_client_close(client);
+    g_free(client->path);
      qemu_mutex_destroy(&client->mutex);
+    qapi_free_NFSServer(client->server);
+    client->server = NULL;
+}
+
+static void nfs_file_close(BlockDriverState *bs)
+{
+    NFSClient *client = bs->opaque;
+    nfs_client_close(client);
  }
static NFSServer *nfs_config(QDict *options, Error **errp)
@@ -498,6 +502,7 @@ static int64_t nfs_client_open(NFSClient *client, QDict 
*options,
      struct stat st;
      char *file = NULL, *strp = NULL;
+ qemu_mutex_init(&client->mutex);
      opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
      qemu_opts_absorb_qdict(opts, options, &local_err);
      if (local_err) {
@@ -660,7 +665,7 @@ static int nfs_file_open(BlockDriverState *bs, QDict 
*options, int flags,
      if (ret < 0) {
          return ret;
      }
-    qemu_mutex_init(&client->mutex);
+
      bs->total_sectors = ret;
      ret = 0;
      return ret;

Reviewed-by: Peter Lieven <address@hidden>

Also CC'ing qemu-stable as this affects 2.9.0 as well.

Peter




reply via email to

[Prev in Thread] Current Thread [Next in Thread]