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@xxxxxxxxxxxxxxxxxxxxxxxxx
> >
> > ------------[ 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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
 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