[syzbot] WARNING in hugetlb_wp

[syzbot] WARNING in hugetlb_wp

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    247f34f7b803 Linux 6.1-rc2
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14a9efd2880000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a66c6c673fb555e8
dashboard link: https://syzkaller.appspot.com/bug?extid=f0b97304ef90f0d0b1dc
compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112217b4880000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=105f4616880000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/de212436b09b/disk-247f34f7.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/63c4feda220f/vmlinux-247f34f7.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 1 PID: 3612 at mm/hugetlb.c:5313 hugetlb_wp+0x20a/0x1af0 mm/hugetlb.c:5313
Modules linked in:
CPU: 1 PID: 3612 Comm: syz-executor250 Not tainted 6.1.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/11/2022
RIP: 0010:hugetlb_wp+0x20a/0x1af0 mm/hugetlb.c:5313
Code: ea 03 80 3c 02 00 0f 85 31 14 00 00 49 8b 5f 20 31 ff 48 89 dd 83 e5 02 48 89 ee e8 70 ab b7 ff 48 85 ed 75 5b e8 76 ae b7 ff <0f> 0b 41 bd 40 00 00 00 e8 69 ae b7 ff 48 b8 00 00 00 00 00 fc ff
RSP: 0018:ffffc90003caf620 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000008640070 RCX: 0000000000000000
RDX: ffff88807b963a80 RSI: ffffffff81c4ed2a RDI: 0000000000000007
RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
R10: 0000000000000000 R11: 000000000008c07e R12: ffff888023805800
R13: 0000000000000000 R14: ffffffff91217f38 R15: ffff88801d4b0360
FS:  0000555555bba300(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fff7a47a1b8 CR3: 000000002378d000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 hugetlb_no_page mm/hugetlb.c:5755 [inline]
 hugetlb_fault+0x19cc/0x2060 mm/hugetlb.c:5874
 follow_hugetlb_page+0x3f3/0x1850 mm/hugetlb.c:6301
 __get_user_pages+0x2cb/0xf10 mm/gup.c:1202
 __get_user_pages_locked mm/gup.c:1434 [inline]
 __get_user_pages_remote+0x18f/0x830 mm/gup.c:2187
 get_user_pages_remote+0x84/0xc0 mm/gup.c:2260
 __access_remote_vm+0x287/0x6b0 mm/memory.c:5517
 ptrace_access_vm+0x181/0x1d0 kernel/ptrace.c:61
 generic_ptrace_pokedata kernel/ptrace.c:1323 [inline]
 ptrace_request+0xb46/0x10c0 kernel/ptrace.c:1046
 arch_ptrace+0x36/0x510 arch/x86/kernel/ptrace.c:828
 __do_sys_ptrace kernel/ptrace.c:1296 [inline]
 __se_sys_ptrace kernel/ptrace.c:1269 [inline]
 __x64_sys_ptrace+0x178/0x2a0 kernel/ptrace.c:1269
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f7f4b262d89
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff7a47a1b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000065
RAX: ffffffffffffffda RBX: 000000000000ab32 RCX: 00007f7f4b262d89
RDX: 00000000200000c0 RSI: 0000000000000e1d RDI: 0000000000000005
RBP: 0000000000000000 R08: 00007fff7a47a358 R09: 00007fff7a47a358
R10: 00000000000003ff R11: 0000000000000246 R12: 00007fff7a47a1cc
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: [syzbot] WARNING in hugetlb_wp

[Mike Kravetz]

On 10/28/22 22:15, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    247f34f7b803 Linux 6.1-rc2
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=14a9efd2880000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=a66c6c673fb555e8
> dashboard link: https://syzkaller.appspot.com/bug?extid=f0b97304ef90f0d0b1dc
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112217b4880000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=105f4616880000
> 
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/de212436b09b/disk-247f34f7.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/63c4feda220f/vmlinux-247f34f7.xz
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com
> 
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 3612 at mm/hugetlb.c:5313 hugetlb_wp+0x20a/0x1af0 mm/hugetlb.c:5313

This warning was added with commit 1d8d14641fd94 ("mm/hugetlb: support write-faults
in shared mappings").  It is there 'by design' as this this specific
type of write fault is not supported.

	/*
	 * hugetlb does not support FOLL_FORCE-style write faults that keep the
	 * PTE mapped R/O such as maybe_mkwrite() would do.
	 */
	if (WARN_ON_ONCE(!unshare && !(vma->vm_flags & VM_WRITE)))
		return VM_FAULT_SIGSEGV;

If there is an actual use case for this support, we can look at adding it.
-- 
Mike Kravetz

Re: [syzbot] WARNING in hugetlb_wp

[David Hildenbrand]

On 30.10.22 02:35, Mike Kravetz wrote:
> On 10/28/22 22:15, syzbot wrote:
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit:    247f34f7b803 Linux 6.1-rc2
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=14a9efd2880000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=a66c6c673fb555e8
>> dashboard link: https://syzkaller.appspot.com/bug?extid=f0b97304ef90f0d0b1dc
>> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112217b4880000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=105f4616880000
>>
>> Downloadable assets:
>> disk image: https://storage.googleapis.com/syzbot-assets/de212436b09b/disk-247f34f7.raw.xz
>> vmlinux: https://storage.googleapis.com/syzbot-assets/63c4feda220f/vmlinux-247f34f7.xz
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 1 PID: 3612 at mm/hugetlb.c:5313 hugetlb_wp+0x20a/0x1af0 mm/hugetlb.c:5313
> 
> This warning was added with commit 1d8d14641fd94 ("mm/hugetlb: support write-faults
> in shared mappings").  It is there 'by design' as this this specific
> type of write fault is not supported.
> 
> 	/*
> 	 * hugetlb does not support FOLL_FORCE-style write faults that keep the
> 	 * PTE mapped R/O such as maybe_mkwrite() would do.
> 	 */
> 	if (WARN_ON_ONCE(!unshare && !(vma->vm_flags & VM_WRITE)))
> 		return VM_FAULT_SIGSEGV;
> 
> If there is an actual use case for this support, we can look at adding it.

Right, it's by design and in retrospective it was the right approach to add this
check there. We seem to have a way to trigger a hugetlb write
fault without VM_WRITE set from GUP. We have to fence that.


Interestingly, I thought I tried to trigger that exact scenario.

a) Have a MAP_PRIVATE, PROT_READ hugetlb mapping
b) Try writing to it via /proc/self/mem, triggering debug access with FOLL_FORCE

The expectation is that this will fail with -EFAULT on hugetlb. I could have
sworn that it did the right thing when I tried :)


But staring at follow_hugetlb_page(), I think we will end up triggering a
write fault (FAULT_FLAG_WRITE) on hugetlb.


The easiest fix might be to special-case hugetlb VMA in check_vma_flags():


 From 39d2a525cae62e7d2766ecfc4337ed4d59555d9d Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Sun, 30 Oct 2022 09:45:50 +0100
Subject: [PATCH] mm/gup: disallow FOLL_FORCE on hugetlb mappings

TODO

Signed-off-by: David Hildenbrand <david@redhat.com>
---
  mm/gup.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index fe195d47de74..b934687efdec 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1076,6 +1076,9 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
  			 */
  			if (!is_cow_mapping(vm_flags))
  				return -EFAULT;
+			/* hugetlb does not support FOLL_FORCE. */
+			if (is_vm_hugetlb_page(vma))
+				return -EFAULT;
  		}
  	} else if (!(vm_flags & VM_READ)) {
  		if (!(gup_flags & FOLL_FORCE))
-- 
2.37.3



-- 
Thanks,

David / dhildenb

Re: [syzbot] WARNING in hugetlb_wp

[David Hildenbrand]

On 30.10.22 09:53, David Hildenbrand wrote:
> On 30.10.22 02:35, Mike Kravetz wrote:
>> On 10/28/22 22:15, syzbot wrote:
>>> Hello,
>>>
>>> syzbot found the following issue on:
>>>
>>> HEAD commit:    247f34f7b803 Linux 6.1-rc2
>>> git tree:       upstream
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=14a9efd2880000
>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=a66c6c673fb555e8
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=f0b97304ef90f0d0b1dc
>>> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112217b4880000
>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=105f4616880000
>>>
>>> Downloadable assets:
>>> disk image: https://storage.googleapis.com/syzbot-assets/de212436b09b/disk-247f34f7.raw.xz
>>> vmlinux: https://storage.googleapis.com/syzbot-assets/63c4feda220f/vmlinux-247f34f7.xz
>>>
>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>> Reported-by: syzbot+f0b97304ef90f0d0b1dc@syzkaller.appspotmail.com
>>>
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 1 PID: 3612 at mm/hugetlb.c:5313 hugetlb_wp+0x20a/0x1af0 mm/hugetlb.c:5313
>>
>> This warning was added with commit 1d8d14641fd94 ("mm/hugetlb: support write-faults
>> in shared mappings").  It is there 'by design' as this this specific
>> type of write fault is not supported.
>>
>> 	/*
>> 	 * hugetlb does not support FOLL_FORCE-style write faults that keep the
>> 	 * PTE mapped R/O such as maybe_mkwrite() would do.
>> 	 */
>> 	if (WARN_ON_ONCE(!unshare && !(vma->vm_flags & VM_WRITE)))
>> 		return VM_FAULT_SIGSEGV;
>>
>> If there is an actual use case for this support, we can look at adding it.
> 
> Right, it's by design and in retrospective it was the right approach to add this
> check there. We seem to have a way to trigger a hugetlb write
> fault without VM_WRITE set from GUP. We have to fence that.
> 
> 
> Interestingly, I thought I tried to trigger that exact scenario.
> 
> a) Have a MAP_PRIVATE, PROT_READ hugetlb mapping
> b) Try writing to it via /proc/self/mem, triggering debug access with FOLL_FORCE
> 
> The expectation is that this will fail with -EFAULT on hugetlb. I could have
> sworn that it did the right thing when I tried :)
> 
> 
> But staring at follow_hugetlb_page(), I think we will end up triggering a
> write fault (FAULT_FLAG_WRITE) on hugetlb.
> 
> 
> The easiest fix might be to special-case hugetlb VMA in check_vma_flags():
> 
> 
>   From 39d2a525cae62e7d2766ecfc4337ed4d59555d9d Mon Sep 17 00:00:00 2001
> From: David Hildenbrand <david@redhat.com>
> Date: Sun, 30 Oct 2022 09:45:50 +0100
> Subject: [PATCH] mm/gup: disallow FOLL_FORCE on hugetlb mappings
> 
> TODO
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>    mm/gup.c | 3 +++
>    1 file changed, 3 insertions(+)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index fe195d47de74..b934687efdec 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1076,6 +1076,9 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
>    			 */
>    			if (!is_cow_mapping(vm_flags))
>    				return -EFAULT;
> +			/* hugetlb does not support FOLL_FORCE. */
> +			if (is_vm_hugetlb_page(vma))
> +				return -EFAULT;
>    		}
>    	} else if (!(vm_flags & VM_READ)) {
>    		if (!(gup_flags & FOLL_FORCE))


A simple reproducer:


#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <stdint.h>
#include <sys/mman.h>
#include <linux/mman.h>

int main(int argc, char **argv)
{
         char *map;
         int mem_fd;

         map = mmap(NULL, 2 * 1024 * 1024u, PROT_READ,
                    MAP_PRIVATE|MAP_ANON|MAP_HUGETLB|MAP_HUGE_2MB, -1, 0);
         if (map == MAP_FAILED) {
                 fprintf(stderr, "mmap() failed: %d\n", errno);
                 return 1;
         }

         mem_fd = open("/proc/self/mem", O_RDWR);
         if (mem_fd < 0) {
                 fprintf(stderr, "open(/proc/self/mem) failed: %d\n", errno);
                 return 1;
         }

         if (pwrite(mem_fd, "0", 1, (uintptr_t) map) == 1) {
                 fprintf(stderr, "write() succeeded, which is unexpected\n");
                 return 1;
         }

         printf("write() failed as expected: %d\n", errno);
         return 0;
}



I started looking at the follow_hugetlb_page() call in __get_user_pages() and
my head seriously started to hurt.

Why are we storing to "i" and error from follow_hugetlb_page()->hugetlb_fault()
and then eventually happily continuing?

I'm afraid of touching that code, it looks too fragile.

Hopefully I am missing something important and it's all perfectly
fine code.


-- 
Thanks,

David / dhildenb