gup: make the stack expansion warning a bit more targeted

I added a warning about about GUP no longer expanding the stack in
commit a425ac5365f6 ("gup: add warning if some caller would seem to want
stack expansion"), but didn't really expect anybody to hit it.

And it's true that nobody seems to have hit a _real_ case yet, but we
certainly have a number of reports of false positives.  Which not only
causes extra noise in itself, but might also end up hiding any real
cases if they do exist.

So let's tighten up the warning condition, and replace the simplistic

	vma = find_vma(mm, start);
	if (vma && (start < vma->vm_start)) {
		WARN_ON_ONCE(vma->vm_flags & VM_GROWSDOWN);

with a

	vma = gup_vma_lookup(mm, start);

helper function which works otherwise like just "vma_lookup()", but with
some heuristics for when to warn about gup no longer causing stack
expansion.

In particular, don't just warn for "below the stack", but warn if it's
_just_ below the stack (with "just below" arbitrarily defined as 64kB,
because why not?).  And rate-limit it to at most once per hour, which
means that any false positives shouldn't completely hide subsequent
reports, but we won't be flooding the logs about it either.

The previous code triggered when some GUP user (chromium crashpad)
accessing past the end of the previous vma, for example.  That has never
expanded the stack, it just causes GUP to return early, and as such we
shouldn't be warning about it.

This is still going trigger the randomized testers, but to mitigate the
noise from that, use "dump_stack()" instead of "WARN_ON_ONCE()" to get
the kernel call chain.  We'll get the relevant information, but syzbot
shouldn't get too upset about it.

Also, don't even bother with the GROWSUP case, which would be using
different heuristics entirely, but only happens on parisc.

Reported-by: kernel test robot <oliver.sang@intel.com>
Reported-by: John Hubbard <jhubbard@nvidia.com>
Reported-by: syzbot+6cf44e127903fdf9d929@syzkaller.appspotmail.com
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/mm/gup.c b/mm/gup.c
index ef29641..76d222c 100644 (file)
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1091,6 +1091,45 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags)
        return 0;
 }
 
+/*
+ * This is "vma_lookup()", but with a warning if we would have
+ * historically expanded the stack in the GUP code.
+ */
+static struct vm_area_struct *gup_vma_lookup(struct mm_struct *mm,
+        unsigned long addr)
+{
+#ifdef CONFIG_STACK_GROWSUP
+       return vma_lookup(mm, addr);
+#else
+       static volatile unsigned long next_warn;
+       struct vm_area_struct *vma;
+       unsigned long now, next;
+
+       vma = find_vma(mm, addr);
+       if (!vma || (addr >= vma->vm_start))
+               return vma;
+
+       /* Only warn for half-way relevant accesses */
+       if (!(vma->vm_flags & VM_GROWSDOWN))
+               return NULL;
+       if (vma->vm_start - addr > 65536)
+               return NULL;
+
+       /* Let's not warn more than once an hour.. */
+       now = jiffies; next = next_warn;
+       if (next && time_before(now, next))
+               return NULL;
+       next_warn = now + 60*60*HZ;
+
+       /* Let people know things may have changed. */
+       pr_warn("GUP no longer grows the stack in %s (%d): %lx-%lx (%lx)\n",
+               current->comm, task_pid_nr(current),
+               vma->vm_start, vma->vm_end, addr);
+       dump_stack();
+       return NULL;
+#endif
+}
+
 /**
  * __get_user_pages() - pin user pages in memory
  * @mm:                mm_struct of target mm
@@ -1168,11 +1207,7 @@ static long __get_user_pages(struct mm_struct *mm,
 
                /* first iteration or cross vma bound */
                if (!vma || start >= vma->vm_end) {
-                       vma = find_vma(mm, start);
-                       if (vma && (start < vma->vm_start)) {
-                               WARN_ON_ONCE(vma->vm_flags & VM_GROWSDOWN);
-                               vma = NULL;
-                       }
+                       vma = gup_vma_lookup(mm, start);
                        if (!vma && in_gate_area(mm, start)) {
                                ret = get_gate_page(mm, start & PAGE_MASK,
                                                gup_flags, &vma,
@@ -1337,13 +1372,9 @@ int fixup_user_fault(struct mm_struct *mm,
                fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
 retry:
-       vma = find_vma(mm, address);
+       vma = gup_vma_lookup(mm, address);
        if (!vma)
                return -EFAULT;
-       if (address < vma->vm_start ) {
-               WARN_ON_ONCE(vma->vm_flags & VM_GROWSDOWN);
-               return -EFAULT;
-       }
 
        if (!vma_permits_fault(vma, fault_flags))
                return -EFAULT;