Re: [PATCH v2 2/3] selftests/memfd_secret: add vmsplice() test

[Mike Rapoport]

On Tue, Mar 26, 2024 at 03:32:09PM +0100, David Hildenbrand wrote:
> Let's add a simple reproducer for a scenario where GUP-fast could succeed
> on secretmem folios, making vmsplice() succeed instead of failing. The
> reproducer is based on a reproducer [1] by Miklos Szeredi.
> 
> We want to perform two tests: vmsplice() when a fresh page was just
> faulted in, and vmsplice() on an existing page after munmap() that
> would drain certain LRU caches/batches in the kernel.
> 
> In an ideal world, we could use fallocate(FALLOC_FL_PUNCH_HOLE) /
> MADV_REMOVE to remove any existing page. As that is currently not
> possible, run the test before any other tests that would allocate memory
> in the secretmem fd.
> 
> Perform the ftruncate() only once, and check the return value.
> 
> [1] https://lkml.kernel.org/r/CAJfpegt3UCsMmxd0taOY11Uaw5U=eS1fE5dn0wZX3HF0oy8-oQ@xxxxxxxxxxxxxx
> 
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>

Reviewed-by: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>

> ---
>  tools/testing/selftests/mm/memfd_secret.c | 51 ++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
> index 9b298f6a04b3..9a0597310a76 100644
> --- a/tools/testing/selftests/mm/memfd_secret.c
> +++ b/tools/testing/selftests/mm/memfd_secret.c
> @@ -20,6 +20,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <stdio.h>
> +#include <fcntl.h>
>  
>  #include "../kselftest.h"
>  
> @@ -83,6 +84,45 @@ static void test_mlock_limit(int fd)
>  	pass("mlock limit is respected\n");
>  }
>  
> +static void test_vmsplice(int fd, const char *desc)
> +{
> +	ssize_t transferred;
> +	struct iovec iov;
> +	int pipefd[2];
> +	char *mem;
> +
> +	if (pipe(pipefd)) {
> +		fail("pipe failed: %s\n", strerror(errno));
> +		return;
> +	}
> +
> +	mem = mmap(NULL, page_size, prot, mode, fd, 0);
> +	if (mem == MAP_FAILED) {
> +		fail("Unable to mmap secret memory\n");
> +		goto close_pipe;
> +	}
> +
> +	/*
> +	 * vmsplice() may use GUP-fast, which must also fail. Prefault the
> +	 * page table, so GUP-fast could find it.
> +	 */
> +	memset(mem, PATTERN, page_size);
> +
> +	iov.iov_base = mem;
> +	iov.iov_len = page_size;
> +	transferred = vmsplice(pipefd[1], &iov, 1, 0);
> +
> +	if (transferred < 0 && errno == EFAULT)
> +		pass("vmsplice is blocked as expected with %s\n", desc);
> +	else
> +		fail("vmsplice: unexpected memory access with %s\n", desc);
> +
> +	munmap(mem, page_size);
> +close_pipe:
> +	close(pipefd[0]);
> +	close(pipefd[1]);
> +}
> +
>  static void try_process_vm_read(int fd, int pipefd[2])
>  {
>  	struct iovec liov, riov;
> @@ -187,7 +227,6 @@ static void test_remote_access(int fd, const char *name,
>  		return;
>  	}
>  
> -	ftruncate(fd, page_size);
>  	memset(mem, PATTERN, page_size);
>  
>  	if (write(pipefd[1], &mem, sizeof(mem)) < 0) {
> @@ -258,7 +297,7 @@ static void prepare(void)
>  				   strerror(errno));
>  }
>  
> -#define NUM_TESTS 4
> +#define NUM_TESTS 6
>  
>  int main(int argc, char *argv[])
>  {
> @@ -277,9 +316,17 @@ int main(int argc, char *argv[])
>  			ksft_exit_fail_msg("memfd_secret failed: %s\n",
>  					   strerror(errno));
>  	}
> +	if (ftruncate(fd, page_size))
> +		ksft_exit_fail_msg("ftruncate failed: %s\n", strerror(errno));
>  
>  	test_mlock_limit(fd);
>  	test_file_apis(fd);
> +	/*
> +	 * We have to run the first vmsplice test before any secretmem page was
> +	 * allocated for this fd.
> +	 */
> +	test_vmsplice(fd, "fresh page");
> +	test_vmsplice(fd, "existing page");
>  	test_process_vm_read(fd);
>  	test_ptrace(fd);
>  
> -- 
> 2.43.2
> 

-- 
Sincerely yours,
Mike.