public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.1] mm: call the security_mmap_file() LSM hook in remap_file_pages()
@ 2024-12-12  3:26 bin.lan.cn
  2024-12-12 17:57 ` Sasha Levin
  0 siblings, 1 reply; 2+ messages in thread
From: bin.lan.cn @ 2024-12-12  3:26 UTC (permalink / raw)
  To: stable, ebpqwerty472123; +Cc: stephen.smalley.work

From: Shu Han <ebpqwerty472123@gmail.com>

[ Upstream commit ea7e2d5e49c05e5db1922387b09ca74aa40f46e2 ]

The remap_file_pages syscall handler calls do_mmap() directly, which
doesn't contain the LSM security check. And if the process has called
personality(READ_IMPLIES_EXEC) before and remap_file_pages() is called for
RW pages, this will actually result in remapping the pages to RWX,
bypassing a W^X policy enforced by SELinux.

So we should check prot by security_mmap_file LSM hook in the
remap_file_pages syscall handler before do_mmap() is called. Otherwise, it
potentially permits an attacker to bypass a W^X policy enforced by
SELinux.

The bypass is similar to CVE-2016-10044, which bypass the same thing via
AIO and can be found in [1].

The PoC:

$ cat > test.c

int main(void) {
	size_t pagesz = sysconf(_SC_PAGE_SIZE);
	int mfd = syscall(SYS_memfd_create, "test", 0);
	const char *buf = mmap(NULL, 4 * pagesz, PROT_READ | PROT_WRITE,
		MAP_SHARED, mfd, 0);
	unsigned int old = syscall(SYS_personality, 0xffffffff);
	syscall(SYS_personality, READ_IMPLIES_EXEC | old);
	syscall(SYS_remap_file_pages, buf, pagesz, 0, 2, 0);
	syscall(SYS_personality, old);
	// show the RWX page exists even if W^X policy is enforced
	int fd = open("/proc/self/maps", O_RDONLY);
	unsigned char buf2[1024];
	while (1) {
		int ret = read(fd, buf2, 1024);
		if (ret <= 0) break;
		write(1, buf2, ret);
	}
	close(fd);
}

$ gcc test.c -o test
$ ./test | grep rwx
7f1836c34000-7f1836c35000 rwxs 00002000 00:01 2050 /memfd:test (deleted)

Link: https://project-zero.issues.chromium.org/issues/42452389 [1]
Cc: stable@vger.kernel.org
Signed-off-by: Shu Han <ebpqwerty472123@gmail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
[PM: subject line tweaks]
Signed-off-by: Paul Moore <paul@paul-moore.com>
[ Resolve merge conflict in mm/mmap.c. ]
Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
---
 mm/mmap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/mmap.c b/mm/mmap.c
index 9a9933ede542..ebc3583fa612 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3021,8 +3021,12 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 		flags |= MAP_LOCKED;
 
 	file = get_file(vma->vm_file);
+	ret = security_mmap_file(vma->vm_file, prot, flags);
+	if (ret)
+		goto out_fput;
 	ret = do_mmap(vma->vm_file, start, size,
 			prot, flags, pgoff, &populate, NULL);
+out_fput:
 	fput(file);
 out:
 	mmap_write_unlock(mm);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 6.1] mm: call the security_mmap_file() LSM hook in remap_file_pages()
  2024-12-12  3:26 [PATCH 6.1] mm: call the security_mmap_file() LSM hook in remap_file_pages() bin.lan.cn
@ 2024-12-12 17:57 ` Sasha Levin
  0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2024-12-12 17:57 UTC (permalink / raw)
  To: stable; +Cc: bin.lan.cn, Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: ea7e2d5e49c05e5db1922387b09ca74aa40f46e2

WARNING: Author mismatch between patch and upstream commit:
Backport author: bin.lan.cn@eng.windriver.com
Commit author: Shu Han <ebpqwerty472123@gmail.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: 49d3a4ad57c5)
6.1.y | Not found

Note: The patch differs from the upstream commit:
---
1:  ea7e2d5e49c05 ! 1:  6fdcac73c2ec9 mm: call the security_mmap_file() LSM hook in remap_file_pages()
    @@ Metadata
      ## Commit message ##
         mm: call the security_mmap_file() LSM hook in remap_file_pages()
     
    +    [ Upstream commit ea7e2d5e49c05e5db1922387b09ca74aa40f46e2 ]
    +
         The remap_file_pages syscall handler calls do_mmap() directly, which
         doesn't contain the LSM security check. And if the process has called
         personality(READ_IMPLIES_EXEC) before and remap_file_pages() is called for
    @@ Commit message
         Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
         [PM: subject line tweaks]
         Signed-off-by: Paul Moore <paul@paul-moore.com>
    +    [ Resolve merge conflict in mm/mmap.c. ]
    +    Signed-off-by: Bin Lan <bin.lan.cn@windriver.com>
     
      ## mm/mmap.c ##
     @@ mm/mmap.c: SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
    @@ mm/mmap.c: SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long
     +	if (ret)
     +		goto out_fput;
      	ret = do_mmap(vma->vm_file, start, size,
    - 			prot, flags, 0, pgoff, &populate, NULL);
    + 			prot, flags, pgoff, &populate, NULL);
     +out_fput:
      	fput(file);
      out:
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-12-12 17:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-12  3:26 [PATCH 6.1] mm: call the security_mmap_file() LSM hook in remap_file_pages() bin.lan.cn
2024-12-12 17:57 ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox