[PATCH] mm/secretmem: prevent uncharged mremap expansion after fork

Daehyeon Ko <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID <[email protected]>
Secretmem mappings are charged against RLIMIT_MEMLOCK and marked
VM_LOCKED because their pages are unevictable and removed from the direct
map.

dup_mmap() clears VM_LOCKED on the child copy, but mremap() uses that
flag to decide whether an expansion needs a memlock limit check and
accounting. An unprivileged child can therefore expand an inherited
secretmem VMA past its limit and populate the added range.

Add a VMA open callback that marks secretmem copies without VM_LOCKED as
VM_DONTEXPAND. dup_mmap() invokes the callback after clearing VM_LOCKED,
while the original charged mapping retains its existing ability to grow
within the limit.

Add a selftest that verifies expansion of an inherited secretmem VMA is
rejected.

Fixes: 1507f51255c9 ("mm: introduce memfd_secret system call to create "secret" memory areas")
Cc: [email protected]
Signed-off-by: Daehyeon Ko <[email protected]>
---
 mm/secretmem.c                            | 11 +++++
 tools/testing/selftests/mm/memfd_secret.c | 58 ++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index 4877c262cb1f6f..e5878ce91c768c 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -108,7 +108,18 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
 	return ret;
 }
 
+static void secretmem_open(struct vm_area_struct *vma)
+{
+	/*
+	 * dup_mmap() clears VM_LOCKED before calling ->open().  Prevent an
+	 * inherited, uncharged mapping from being expanded by mremap().
+	 */
+	if (!vma_test(vma, VMA_LOCKED_BIT))
+		vma_set_flags(vma, VMA_DONTEXPAND_BIT);
+}
+
 static const struct vm_operations_struct secretmem_vm_ops = {
+	.open = secretmem_open,
 	.fault = secretmem_fault,
 };
 
diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c
index aac4f795c327bd..3d33487eaaccf3 100644
--- a/tools/testing/selftests/mm/memfd_secret.c
+++ b/tools/testing/selftests/mm/memfd_secret.c
@@ -84,6 +84,61 @@ static void test_mlock_limit(int fd)
 	pass("mlock limit is respected\n");
 }
 
+static void test_mremap_after_fork(void)
+{
+	void *mem, *remapped;
+	pid_t pid, waited;
+	int fd, status;
+
+	fd = memfd_secret(0);
+	if (fd < 0) {
+		fail("memfd_secret failed: %s\n", strerror(errno));
+		return;
+	}
+
+	if (ftruncate(fd, page_size * 2)) {
+		fail("ftruncate failed: %s\n", strerror(errno));
+		goto close_fd;
+	}
+
+	mem = mmap(NULL, page_size, prot, mode, fd, 0);
+	if (mem == MAP_FAILED) {
+		fail("unable to mmap secret memory: %s\n", strerror(errno));
+		goto close_fd;
+	}
+
+	pid = fork();
+	if (pid < 0) {
+		fail("fork failed: %s\n", strerror(errno));
+		goto unmap;
+	}
+
+	if (pid == 0) {
+		remapped = mremap(mem, page_size, page_size * 2,
+				  MREMAP_MAYMOVE);
+		if (remapped != MAP_FAILED) {
+			munmap(remapped, page_size * 2);
+			_exit(KSFT_FAIL);
+		}
+		_exit(errno == EFAULT ? KSFT_PASS : KSFT_FAIL);
+	}
+
+	do {
+		waited = waitpid(pid, &status, 0);
+	} while (waited < 0 && errno == EINTR);
+
+	if (waited == pid && WIFEXITED(status) &&
+	    WEXITSTATUS(status) == KSFT_PASS)
+		pass("mremap expansion after fork is blocked\n");
+	else
+		fail("mremap expansion after fork was not blocked\n");
+
+unmap:
+	munmap(mem, page_size);
+close_fd:
+	close(fd);
+}
+
 static void test_vmsplice(int fd, const char *desc)
 {
 	ssize_t transferred;
@@ -297,7 +352,7 @@ static void prepare(void)
 				   strerror(errno));
 }
 
-#define NUM_TESTS 6
+#define NUM_TESTS 7
 
 int main(int argc, char *argv[])
 {
@@ -320,6 +375,7 @@ int main(int argc, char *argv[])
 		ksft_exit_fail_msg("ftruncate failed: %s\n", strerror(errno));
 
 	test_mlock_limit(fd);
+	test_mremap_after_fork();
 	test_file_apis(fd);
 	/*
 	 * We have to run the first vmsplice test before any secretmem page was
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.