[Qemu-devel] [PATCH 0/2] file-posix: Fix creation locking

[Qemu-devel] [PATCH 0/2] file-posix: Fix creation locking

[Max Reitz]

This series fixes two bugs Kevin spotted in file-posix's creation
locking.


Max Reitz (2):
  file-posix: Fix creation locking
  file-posix: Unlock FD after creation

 block/file-posix.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] file-posix: Fix creation locking

[Max Reitz]

raw_apply_lock_bytes() takes a bit mask of "permissions that are NOT
shared".

Also, make the "perm" and "shared" variables uint64_t, because I do not
particularly like using ~ on signed integers (and other permission masks
are usually uint64_t, too).

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index e04e464e72..32b1413c7d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2112,7 +2112,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
 {
     BlockdevCreateOptionsFile *file_opts;
     int fd;
-    int perm, shared;
+    uint64_t perm, shared;
     int result = 0;
 
     /* Validate options and set default values */
@@ -2148,7 +2148,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
 
     /* Step one: Take locks */
-    result = raw_apply_lock_bytes(fd, perm, shared, false, errp);
+    result = raw_apply_lock_bytes(fd, perm, ~shared, false, errp);
     if (result < 0) {
         goto out_close;
     }
-- 
2.17.1

[Qemu-devel] [PATCH 2/2] file-posix: Unlock FD after creation

[Max Reitz]

Closing the FD does not necessarily mean that it is unlocked.  Fix this
by relinquishing all permission locks before qemu_close().

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 32b1413c7d..349f77a3af 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2111,6 +2111,7 @@ static int coroutine_fn
 raw_co_create(BlockdevCreateOptions *options, Error **errp)
 {
     BlockdevCreateOptionsFile *file_opts;
+    Error *local_err = NULL;
     int fd;
     uint64_t perm, shared;
     int result = 0;
@@ -2156,13 +2157,13 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     /* Step two: Check that nobody else has taken conflicting locks */
     result = raw_check_lock_bytes(fd, perm, shared, errp);
     if (result < 0) {
-        goto out_close;
+        goto out_unlock;
     }
 
     /* Clear the file by truncating it to 0 */
     result = raw_regular_truncate(NULL, fd, 0, PREALLOC_MODE_OFF, errp);
     if (result < 0) {
-        goto out_close;
+        goto out_unlock;
     }
 
     if (file_opts->nocow) {
@@ -2185,7 +2186,17 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     result = raw_regular_truncate(NULL, fd, file_opts->size,
                                   file_opts->preallocation, errp);
     if (result < 0) {
-        goto out_close;
+        goto out_unlock;
+    }
+
+out_unlock:
+    raw_apply_lock_bytes(fd, 0, 0, true, &local_err);
+    if (local_err) {
+        /* The above call should not fail, and if it does, that does
+         * not mean the whole creation operation has failed.  So
+         * report it the user for their convenience, but do not report
+         * it to the caller. */
+        error_report_err(local_err);
     }
 
 out_close:
-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/2] file-posix: Fix creation locking

[Kevin Wolf]

Am 04.07.2018 um 16:47 hat Max Reitz geschrieben:
> This series fixes two bugs Kevin spotted in file-posix's creation
> locking.

Thanks, applied to the block branch.

Kevin