lkml.org 
[lkml]   [2011]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] mm: fix possible cause of a page_mapped BUG
    On Wed, 6 Apr 2011, Linus Torvalds wrote:
    > On Wed, Apr 6, 2011 at 7:47 AM, Hugh Dickins <hughd@google.com> wrote:
    > >>
    > >> I dunno. But that odd negative pg_off thing makes me think there is
    > >> some overflow issue (ie HEAP_INDEX being pg_off + size ends up
    > >> fluctuating between really big and really small). So I'd suspect THAT
    > >> as the main reason.
    > >
    > > Yes, one of the vmas is such that the end offset (pgoff of next page
    > > after) would be 0, and for the other it would be 16.  There's sure to
    > > be places, inside the prio_tree code and outside it, where we rely
    > > upon pgoff not wrapping around - wrap should be prevented by original
    > > validation of arguments.
    >
    > Well, we _do_ validate them in do_mmap_pgoff(), which is the main
    > routine for all the mmap() system calls, and the main way to get a new
    > mapping.
    >
    > There are other ways, like do_brk(), but afaik that always sets
    > vm_pgoff to the virtual address (shifted), so again the new mapping
    > should be fine.
    >
    > So when a new mapping is created, it should all be ok.
    >
    > But I think mremap() may end up expanding it without doing the same
    > overflow check.
    >
    > Do you see any other way to get this situation? Does the vma dump give
    > you any hint about where it came from?
    >
    > Robert - here's a (UNTESTED!) patch to make mremap() be a bit more
    > careful about vm_pgoff when growing a mapping. Does it make any
    > difference?

    I'd come to the same conclusion: the original page_mapped BUG has itself
    suggested that mremap() is getting used.

    I was about to send you my own UNTESTED patch: let me append it anyway,
    I think it is more correct than yours (it's the offset of vm_end we need
    to worry about, and there's the funny old_len,new_len stuff). See what
    you think - sorry, I'm going out now.

    Hugh

    --- 2.6.38/mm/mremap.c 2011-03-14 18:20:32.000000000 -0700
    +++ linux/mm/mremap.c 2011-04-06 08:31:46.000000000 -0700
    @@ -282,6 +282,12 @@ static struct vm_area_struct *vma_to_res
    goto Efault;
    }

    + if (vma->vm_file && new_len > old_len) {
    + pgoff_t endoff = linear_page_index(vma, vma->vm_end);
    + if (endoff + ((new_len - old_len) >> PAGE_SHIFT) < endoff)
    + goto Eoverflow;
    + }
    +
    if (vma->vm_flags & VM_LOCKED) {
    unsigned long locked, lock_limit;
    locked = mm->locked_vm << PAGE_SHIFT;
    @@ -311,6 +317,8 @@ Enomem:
    return ERR_PTR(-ENOMEM);
    Eagain:
    return ERR_PTR(-EAGAIN);
    +Eoverflow:
    + return ERR_PTR(-EOVERFLOW);
    }

    static unsigned long mremap_to(unsigned long addr,
    \
     
     \ /
      Last update: 2011-04-06 17:47    [W:0.428 / U:0.060 seconds]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean