Re: [PATCH RFC v2 10/50] KVM: selftests: Test rejection of swap tmpfs mounts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: missing CAP_SYS_ADMIN check in create_tmpfs_pool_fd()
--

commit 81fb5dc552f2fa0364e094d89b7aaec3df677efb
Author: Ackerley Tng <ackerleytng@google.com>

KVM: selftests: Test rejection of swap tmpfs mounts

This commit adds a helper to mount a tmpfs instance as a resource pool fd and
tests that creating a guest_memfd backed by a swap-enabled tmpfs mount is
rejected with -EINVAL.

> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index d62ae23256083..205f822e10c6c 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c

[ ... ]

> @@ -478,6 +480,73 @@ static void test_resource_fd_memfd(struct kvm_vm *vm)
>  	close(file_fd);
>  }
>  
> +static int create_tmpfs_pool_fd(const char *huge, bool noswap, size_t size)
> +{
> +	char size_str[32];
> +	char template[] = "/tmp/gmem_test_dir_XXXXXX";
> +	char *mount_dir;
> +	int fs_fd, mnt_fd, dir_fd;
> +
> +	TEST_ASSERT(size && IS_ALIGNED(size, page_size),
> +		    "Pool size '0x%zx' must be positive and page-aligned", size);
> +
> +	fs_fd = syscall(__NR_fsopen, "tmpfs", FSOPEN_CLOEXEC);
> +	TEST_ASSERT(fs_fd >= 0, "fsopen failed");
> +
> +	if (noswap)
> +		TEST_ASSERT(!syscall(__NR_fsconfig, fs_fd, FSCONFIG_SET_FLAG,
> +				     "noswap", NULL, 0), "fsconfig noswap failed");

[Severity: Medium]
What happens in tools/testing/selftests/kvm/guest_memfd_test.c when
guest_memfd_test is run by an unprivileged user who has access to /dev/kvm
but lacks CAP_SYS_ADMIN?

Through the call chain:

  main()
    test_guest_memfd()
      test_resource_fd_tmpfs_swap()
        create_tmpfs_pool_fd("never", false, page_size)

syscall(__NR_fsopen, "tmpfs", FSOPEN_CLOEXEC) checks may_mount() in the
kernel and fails with -EPERM without CAP_SYS_ADMIN (and when noswap is true,
shmem_parse_one() for "noswap" also requires capable(CAP_SYS_ADMIN) and fails
with -EINVAL).

Because test_assert() in tools/testing/selftests/kvm/lib/assert.c only skips
on EACCES and exits with code 254 on EPERM, doesn't TEST_ASSERT(fs_fd >= 0,
...) cause the entire test binary to fail and abort in test_guest_memfd()
before running __test_guest_memfd() and the non-tmpfs guest_memfd tests?

Also, a later patch in this series ("KVM: selftests: Test guest execution
with tmpfs-backed guest_memfd") checks:

  if (pool_fd < 0)
          TEST_REQUIRE(false);

after calling create_tmpfs_pool_fd(), which appears to be unreachable as long
as create_tmpfs_pool_fd() aborts via TEST_ASSERT() instead of returning an
error or skipping when CAP_SYS_ADMIN is missing. Later in the series ("KVM:
selftests: Test guest_memfd conversions with tmpfs backing"),
guest_memfd_conversions_test will also hit the same failure when run without
CAP_SYS_ADMIN.

Should create_tmpfs_pool_fd() and its callers skip the tmpfs resource pool
tests when the process lacks CAP_SYS_ADMIN, similar to how test_mbind() and
test_numa_allocation() skip on EPERM?

[ ... ]

> @@ -539,6 +608,7 @@ static void test_guest_memfd(unsigned long vm_type)
>  	test_resource_fd_invalid(vm);
>  	test_resource_fd_unsupported_fs(vm);
>  	test_resource_fd_memfd(vm);
> +	test_resource_fd_tmpfs_swap(vm);
>  
>  	__test_guest_memfd(vm, 0);
>  

титься

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-gmem-alternative-providers-v2-0-5dd1dc6c660a@google.com?part=10