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