Commit 166c7edb authored by Michal Hocko's avatar Michal Hocko Committed by Kleber Sacilotto de Souza

mm: do not bug_on on incorrect length in __mm_populate()

BugLink: https://bugs.launchpad.net/bugs/1810947

commit bb177a73 upstream.

syzbot has noticed that a specially crafted library can easily hit
VM_BUG_ON in __mm_populate

  kernel BUG at mm/gup.c:1242!
  invalid opcode: 0000 [#1] SMP
  CPU: 2 PID: 9667 Comm: a.out Not tainted 4.18.0-rc3 #644
  Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
  RIP: 0010:__mm_populate+0x1e2/0x1f0
  Code: 55 d0 65 48 33 14 25 28 00 00 00 89 d8 75 21 48 83 c4 20 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 75 18 f1 ff 0f 0b e8 6e 18 f1 ff <0f> 0b 31 db eb c9 e8 93 06 e0 ff 0f 1f 00 55 48 89 e5 53 48 89 fb
  Call Trace:
     vm_brk_flags+0xc3/0x100
     vm_brk+0x1f/0x30
     load_elf_library+0x281/0x2e0
     __ia32_sys_uselib+0x170/0x1e0
     do_fast_syscall_32+0xca/0x420
     entry_SYSENTER_compat+0x70/0x7f

The reason is that the length of the new brk is not page aligned when we
try to populate the it.  There is no reason to bug on that though.
do_brk_flags already aligns the length properly so the mapping is
expanded as it should.  All we need is to tell mm_populate about it.
Besides that there is absolutely no reason to to bug_on in the first
place.  The worst thing that could happen is that the last page wouldn't
get populated and that is far from putting system into an inconsistent
state.

Fix the issue by moving the length sanitization code from do_brk_flags
up to vm_brk_flags.  The only other caller of do_brk_flags is brk
syscall entry and it makes sure to provide the proper length so t here
is no need for sanitation and so we can use do_brk_flags without it.

Also remove the bogus BUG_ONs.

[osalvador@techadventures.net: fix up vm_brk_flags s@request@len@]
Link: http://lkml.kernel.org/r/20180706090217.GI32658@dhcp22.suse.czSigned-off-by: default avatarMichal Hocko <mhocko@suse.com>
Reported-by: default avatarsyzbot <syzbot+5dcb560fe12aa5091c06@syzkaller.appspotmail.com>
Tested-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: default avatarOscar Salvador <osalvador@suse.de>
Cc: Zi Yan <zi.yan@cs.rutgers.edu>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: "Huang, Ying" <ying.huang@intel.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
[bwh: Backported to 4.4:
 - There is no do_brk_flags() function; update do_brk()
 - do_brk(), vm_brk() return the address on success
 - Adjust context]
Signed-off-by: default avatarBen Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent ad3d72a8
...@@ -940,8 +940,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors) ...@@ -940,8 +940,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors)
int locked = 0; int locked = 0;
long ret = 0; long ret = 0;
VM_BUG_ON(start & ~PAGE_MASK);
VM_BUG_ON(len != PAGE_ALIGN(len));
end = start + len; end = start + len;
for (nstart = start; nstart < end; nstart = nend) { for (nstart = start; nstart < end; nstart = nend) {
......
...@@ -2841,21 +2841,15 @@ static inline void verify_mm_writelocked(struct mm_struct *mm) ...@@ -2841,21 +2841,15 @@ static inline void verify_mm_writelocked(struct mm_struct *mm)
* anonymous maps. eventually we may be able to do some * anonymous maps. eventually we may be able to do some
* brk-specific accounting here. * brk-specific accounting here.
*/ */
static unsigned long do_brk(unsigned long addr, unsigned long request) static unsigned long do_brk(unsigned long addr, unsigned long len)
{ {
struct mm_struct *mm = current->mm; struct mm_struct *mm = current->mm;
struct vm_area_struct *vma, *prev; struct vm_area_struct *vma, *prev;
unsigned long flags, len; unsigned long flags;
struct rb_node **rb_link, *rb_parent; struct rb_node **rb_link, *rb_parent;
pgoff_t pgoff = addr >> PAGE_SHIFT; pgoff_t pgoff = addr >> PAGE_SHIFT;
int error; int error;
len = PAGE_ALIGN(request);
if (len < request)
return -ENOMEM;
if (!len)
return addr;
flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED); error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
...@@ -2923,12 +2917,19 @@ static unsigned long do_brk(unsigned long addr, unsigned long request) ...@@ -2923,12 +2917,19 @@ static unsigned long do_brk(unsigned long addr, unsigned long request)
return addr; return addr;
} }
unsigned long vm_brk(unsigned long addr, unsigned long len) unsigned long vm_brk(unsigned long addr, unsigned long request)
{ {
struct mm_struct *mm = current->mm; struct mm_struct *mm = current->mm;
unsigned long len;
unsigned long ret; unsigned long ret;
bool populate; bool populate;
len = PAGE_ALIGN(request);
if (len < request)
return -ENOMEM;
if (!len)
return addr;
down_write(&mm->mmap_sem); down_write(&mm->mmap_sem);
ret = do_brk(addr, len); ret = do_brk(addr, len);
populate = ((mm->def_flags & VM_LOCKED) != 0); populate = ((mm->def_flags & VM_LOCKED) != 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment