| Message ID | 20240326143210.291116-3-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/secretmem: one fix and one refactoring | expand |
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@mail.gmail.com > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org> > --- > 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 >
On Tue, Mar 26, 2024 at 3:32 PM David Hildenbrand <david@redhat.com> 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@mail.gmail.com > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > 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); Shouldn't the non-prefault case be tested as well? Thanks, Miklos
>> +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); > > Shouldn't the non-prefault case be tested as well? That's the "easy" case where GUP-fast is never involved, and it should mostly be covered by the ptrace/process_vm_read() tests already.
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);
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@mail.gmail.com Signed-off-by: David Hildenbrand <david@redhat.com> --- tools/testing/selftests/mm/memfd_secret.c | 51 ++++++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-)