lkml.org 
[lkml]   [2015]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] mm/mmap.c: remove incorrect MAP_FIXED flag comparison from mmap_region
    On Tue, Nov 17, 2015 at 04:52:51PM -0800, Andrew Morton wrote:
    > On Tue, 17 Nov 2015 18:26:38 +0100 Piotr Kwapulinski <kwapulinski.piotr@gmail.com> wrote:
    >
    > > The following flag comparison in mmap_region is not fully correct:
    > >
    > > if (!(vm_flags & MAP_FIXED))
    > >
    > > The vm_flags should not be compared with MAP_FIXED (0x10). It is a bit
    > > confusing. This condition is almost always true since VM_MAYREAD (0x10)
    > > flag is almost always set by default. This patch removes this condition.
    > >
    > > ...
    > >
    > > --- a/mm/mmap.c
    > > +++ b/mm/mmap.c
    > > @@ -1547,13 +1547,6 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
    > > if (!may_expand_vm(mm, len >> PAGE_SHIFT)) {
    > > unsigned long nr_pages;
    > >
    > > - /*
    > > - * MAP_FIXED may remove pages of mappings that intersects with
    > > - * requested mapping. Account for the pages it would unmap.
    > > - */
    > > - if (!(vm_flags & MAP_FIXED))
    > > - return -ENOMEM;
    > > -
    > > nr_pages = count_vma_pages_range(mm, addr, addr + len);
    > >
    > > if (!may_expand_vm(mm, (len >> PAGE_SHIFT) - nr_pages))
    >
    > That looks simpler.
    >
    > However the changelog doesn't describe the end-user visible effects of
    > the bug, as changelogs should always do. Presumably this is causing
    > incorrect ENOMEM reporting due to RLIMIT_AS being exceeded, but this
    > isn't very specific.
    >
    > So can you please fill in the details here? Such info is needed when
    > deciding which kernel version(s) need the fix.
    >
    > Thanks.

    The first patch has got a user visible effect and it fixes the
    real issue (corner case one). The second patch has no user visible effect.
    It just removes the code that makes no sense. The second patch has
    been created in case the first patch was not going to be accepted.
    I will send both patches again to let you choose which one you preffer.
    This time the patches will contain the more clear changelog containing
    the user visible effect.

    Thanks.
    ---
    Piotr Kwapulinski


    \
     
     \ /
      Last update: 2015-11-18 18:01    [W:2.789 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site