diff mbox series

[1/2] block: Add BDRV_O_NO_SHARE for blk_new_open()

Message ID 20210413132324.24043-2-kwolf@redhat.com
State New
Headers show
Series qemu-img convert: Unshare write permission for source | expand

Commit Message

Kevin Wolf April 13, 2021, 1:23 p.m. UTC 
Normally, blk_new_open() just shares all permissions. This was fine
originally when permissions only protected against uses in the same
process because no other part of the code would actually get to access
the block nodes opened with blk_new_open(). However, since we use it for
file locking now, unsharing permissions becomes desirable.

Add a new BDRV_O_NO_SHARE flag that is used in blk_new_open() to unshare
any permissions that can be unshared.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block.h |  1 +
 block/block-backend.c | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Eric Blake April 13, 2021, 1:41 p.m. UTC | #1 
On 4/13/21 8:23 AM, Kevin Wolf wrote:
> Normally, blk_new_open() just shares all permissions. This was fine
> originally when permissions only protected against uses in the same
> process because no other part of the code would actually get to access
> the block nodes opened with blk_new_open(). However, since we use it for
> file locking now, unsharing permissions becomes desirable.
> 
> Add a new BDRV_O_NO_SHARE flag that is used in blk_new_open() to unshare
> any permissions that can be unshared.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/block.h |  1 +
>  block/block-backend.c | 19 +++++++++++++------
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/include/block/block.h b/include/block/block.h
> index b3f6e509d4..735db05a39 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -101,6 +101,7 @@ typedef struct HDGeometry {
>      uint32_t cylinders;
>  } HDGeometry;
>  
> +#define BDRV_O_NO_SHARE    0x0001 /* don't share permissons */
>  #define BDRV_O_RDWR        0x0002

Huh. We originally created BDRV_O_RDONLY = 0 and BDRV_O_RDWR = 2 back in
commit 83f6409109 (in 2006); I see no reason why we skipped 1 at that
time (maybe to allow BDRV_O_WRONLY? but who would use it), so exploiting
it now seems fine.

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

Patch

diff --git a/include/block/block.h b/include/block/block.h
index b3f6e509d4..735db05a39 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -101,6 +101,7 @@  typedef struct HDGeometry {
     uint32_t cylinders;
 } HDGeometry;
 
+#define BDRV_O_NO_SHARE    0x0001 /* don't share permissons */
 #define BDRV_O_RDWR        0x0002
 #define BDRV_O_RESIZE      0x0004 /* request permission for resizing the node */
 #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
diff --git a/block/block-backend.c b/block/block-backend.c
index 413af51f3b..b4746541ac 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -398,15 +398,19 @@  BlockBackend *blk_new_open(const char *filename, const char *reference,
     BlockBackend *blk;
     BlockDriverState *bs;
     uint64_t perm = 0;
+    uint64_t shared = BLK_PERM_ALL;
 
-    /* blk_new_open() is mainly used in .bdrv_create implementations and the
-     * tools where sharing isn't a concern because the BDS stays private, so we
-     * just request permission according to the flags.
+    /*
+     * blk_new_open() is mainly used in .bdrv_create implementations and the
+     * tools where sharing isn't a major concern because the BDS stays private
+     * and the file is generally not supposed to be used by a second process,
+     * so we just request permission according to the flags.
      *
      * The exceptions are xen_disk and blockdev_init(); in these cases, the
      * caller of blk_new_open() doesn't make use of the permissions, but they
      * shouldn't hurt either. We can still share everything here because the
-     * guest devices will add their own blockers if they can't share. */
+     * guest devices will add their own blockers if they can't share.
+     */
     if ((flags & BDRV_O_NO_IO) == 0) {
         perm |= BLK_PERM_CONSISTENT_READ;
         if (flags & BDRV_O_RDWR) {
@@ -416,8 +420,11 @@  BlockBackend *blk_new_open(const char *filename, const char *reference,
     if (flags & BDRV_O_RESIZE) {
         perm |= BLK_PERM_RESIZE;
     }
+    if (flags & BDRV_O_NO_SHARE) {
+        shared = BLK_PERM_WRITE_UNCHANGED;
+    }
 
-    blk = blk_new(qemu_get_aio_context(), perm, BLK_PERM_ALL);
+    blk = blk_new(qemu_get_aio_context(), perm, shared);
     bs = bdrv_open(filename, reference, options, flags, errp);
     if (!bs) {
         blk_unref(blk);
@@ -426,7 +433,7 @@  BlockBackend *blk_new_open(const char *filename, const char *reference,
 
     blk->root = bdrv_root_attach_child(bs, "root", &child_root,
                                        BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
-                                       blk->ctx, perm, BLK_PERM_ALL, blk, errp);
+                                       blk->ctx, perm, shared, blk, errp);
     if (!blk->root) {
         blk_unref(blk);
         return NULL;