[PATCH 5.10] mm: call the security_mmap_file() LSM hook in remap_file_pages()

[PATCH 5.10] mm: call the security_mmap_file() LSM hook in remap_file_pages()

[Pratyush Yadav]

From: Shu Han <ebpqwerty472123@gmail.com>

commit ea7e2d5e49c05e5db1922387b09ca74aa40f46e2 upstream.

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>
Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
---
 mm/mmap.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/mmap.c b/mm/mmap.c
index 9f76625a1743..2c17eb840e44 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3078,8 +3078,12 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 	}
 
 	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.47.1

Re: [PATCH 5.10] mm: call the security_mmap_file() LSM hook in remap_file_pages()

[Greg Kroah-Hartman]

On Mon, Feb 10, 2025 at 07:10:54PM +0000, Pratyush Yadav wrote:
> From: Shu Han <ebpqwerty472123@gmail.com>
> 
> commit ea7e2d5e49c05e5db1922387b09ca74aa40f46e2 upstream.
> 
> 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>
> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
> ---
>  mm/mmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 9f76625a1743..2c17eb840e44 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3078,8 +3078,12 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
>  	}
>  
>  	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.47.1
> 
> 

This has required fixes for this commit which you did not include here,
so I'm going to have to drop this from the tree.  Same for the other
branch you submitted this against.

Please be more careful and always include all needed commits to resolve
a problem, we don't want to purposfully add bugs to the kernel tree that
we have already resolved.

thanks,

greg k-h

Re: [PATCH 5.10] mm: call the security_mmap_file() LSM hook in remap_file_pages()

[Pratyush Yadav]

On Wed, Feb 19 2025, Greg Kroah-Hartman wrote:

> On Mon, Feb 10, 2025 at 07:10:54PM +0000, Pratyush Yadav wrote:
>> From: Shu Han <ebpqwerty472123@gmail.com>
>>
>> commit ea7e2d5e49c05e5db1922387b09ca74aa40f46e2 upstream.
>>
>> 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>
>> Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
>> ---
>>  mm/mmap.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/mm/mmap.c b/mm/mmap.c
>> index 9f76625a1743..2c17eb840e44 100644
>> --- a/mm/mmap.c
>> +++ b/mm/mmap.c
>> @@ -3078,8 +3078,12 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
>>       }
>>
>>       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.47.1
>>
>>
>
> This has required fixes for this commit which you did not include here,
> so I'm going to have to drop this from the tree.  Same for the other
> branch you submitted this against.
>
> Please be more careful and always include all needed commits to resolve
> a problem, we don't want to purposfully add bugs to the kernel tree that
> we have already resolved.

My bad. I wanted to fix the CVE assigned to this patch and I didn't
think of looking for follow-up fixes. Will do that next time around.

-- 
Regards,
Pratyush Yadav