qemu-block
[Top][All Lists]
Advanced

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

Re: [PULL 10/28] export/fuse: Pass default_permissions for mount


From: Hanna Reitz
Subject: Re: [PULL 10/28] export/fuse: Pass default_permissions for mount
Date: Mon, 3 Jan 2022 11:04:52 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

On 24.12.21 16:04, Vladimir Sementsov-Ogievskiy wrote:
09.07.2021 15:50, Kevin Wolf wrote:
From: Max Reitz <mreitz@redhat.com>

We do not do any permission checks in fuse_open(), so let the kernel do
them.  We already let fuse_getattr() report the proper UNIX permissions,
so this should work the way we want.

This causes a change in 308's reference output, because now opening a
non-writable export with O_RDWR fails already, instead of only actually
attempting to write to it.  (That is an improvement.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210625142317.271673-2-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
  block/export/fuse.c        | 8 ++++++--
  tests/qemu-iotests/308     | 3 ++-
  tests/qemu-iotests/308.out | 2 +-
  3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/block/export/fuse.c b/block/export/fuse.c
index 38f74c94da..d0b88e8f80 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -153,8 +153,12 @@ static int setup_fuse_export(FuseExport *exp, const char *mountpoint,
      struct fuse_args fuse_args;
      int ret;
  -    /* Needs to match what fuse_init() sets.  Only max_read must be supplied. */ -    mount_opts = g_strdup_printf("max_read=%zu", FUSE_MAX_BOUNCE_BYTES);
+    /*
+     * max_read needs to match what fuse_init() sets.
+     * max_write need not be supplied.
+     */
+    mount_opts = g_strdup_printf("max_read=%zu,default_permissions",
+                                 FUSE_MAX_BOUNCE_BYTES);
        fuse_argv[0] = ""; /* Dummy program name */
      fuse_argv[1] = "-o";
diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308
index f122065d0f..11c28a75f2 100755
--- a/tests/qemu-iotests/308
+++ b/tests/qemu-iotests/308
@@ -215,7 +215,8 @@ echo '=== Writable export ==='
  fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP', 'writable': true"
    # Check that writing to the read-only export fails
-$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" 2>&1 \
+    | _filter_qemu_io | _filter_testdir | _filter_imgfmt
    # But here it should work
  $QEMU_IO -f raw -c 'write -P 42 1M 64k' "$EXT_MP" | _filter_qemu_io
diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out
index 466e7e0267..0e9420645f 100644
--- a/tests/qemu-iotests/308.out
+++ b/tests/qemu-iotests/308.out
@@ -91,7 +91,7 @@ virtual size: 0 B (0 bytes)
                'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true
            } }
  {"return": {}}
-write failed: Permission denied
+qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
  wrote 65536/65536 bytes at offset 1048576
  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  wrote 65536/65536 bytes at offset 1048576


Hi!

308 test fails for me now:

308   fail       [16:02:49] [16:02:53]   3.5s   (last: 3.7s) output mismatch (see 308.out.bad)
--- /work/src/qemu/master/tests/qemu-iotests/308.out
+++ 308.out.bad
@@ -91,7 +91,7 @@
               'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true
           } }
 {"return": {}}
-qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
+write failed: Permission denied
 wrote 65536/65536 bytes at offset 1048576
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset 1048576
Failures: 308
Failed 1 of 1 iotests


And bisect points to exactly this commit.

Should it somehow depend on environment?

I suspect that’s because you’re running the test as root? (CAP_DAC_OVERRIDE allows opening files even when the permissions don’t allow for it.  We already have similar cases in 118.)

Skipping the whole test for root would be quite harsh...  Since this is just about a single line, I think we can do something with .casenotrun.

Hanna




reply via email to

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