Hi David, On Mon, Mar 25, 2024 at 02:41:13PM +0100, David Hildenbrand wrote: > Let's add a simple reproducer for a scneario 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. > > Perform the ftruncate() only once, and check the return value. > > For some reason, vmsplice() reliably fails (making the test succeed) when > we move the test_vmsplice() call after test_process_vm_read() / > test_ptrace(). That's because ftruncate() call was in test_remote_access() and you need it to mmap secretmem. > Properly cleaning up in test_remote_access(), which is not > part of this change, won't change that behavior. Therefore, run the > vmsplice() test for now first -- something is a bit off once we involve > fork(). > > [1] https://lkml.kernel.org/r/CAJfpegt3UCsMmxd0taOY11Uaw5U=eS1fE5dn0wZX3HF0oy8-oQ@xxxxxxxxxxxxxx > > Signed-off-by: David Hildenbrand <david@xxxxxxxxxx> > --- > tools/testing/selftests/mm/memfd_secret.c | 44 +++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c > index 9b298f6a04b3..0acbdcf8230e 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,43 @@ static void test_mlock_limit(int fd) > pass("mlock limit is respected\n"); > } > > +static void test_vmsplice(int fd) > +{ > + 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); > + > + ksft_test_result(transferred < 0 && errno == EFAULT, > + "vmsplice is blocked as expected\n"); The same message will be printed on success and on failure. I think if (transferred < 0 && errno == EFAULT) pass("vmsplice is blocked as expected"); else fail("vmsplice: unexpected memory acccess"); is clearer than feeding different strings to ksft_test_result(). Other than that Reviewed-by: Mike Rapoport (IBM) <rppt@xxxxxxxxxx> > + > + munmap(mem, page_size); > +close_pipe: > + close(pipefd[0]); > + close(pipefd[1]); > +} -- Sincerely yours, Mike.
