Skip to content

Commit bb177a7

Browse files
Michal Hockotorvalds
authored andcommitted
mm: do not bug_on on incorrect length in __mm_populate()
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. [[email protected]: fix up vm_brk_flags s@request@len@] Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Michal Hocko <[email protected]> Reported-by: syzbot <[email protected]> Tested-by: Tetsuo Handa <[email protected]> Reviewed-by: Oscar Salvador <[email protected]> Cc: Zi Yan <[email protected]> Cc: "Aneesh Kumar K.V" <[email protected]> Cc: Dan Williams <[email protected]> Cc: "Kirill A. Shutemov" <[email protected]> Cc: Michael S. Tsirkin <[email protected]> Cc: Al Viro <[email protected]> Cc: "Huang, Ying" <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent e3d301c commit bb177a7

File tree

2 files changed

+12
-19
lines changed

2 files changed

+12
-19
lines changed

mm/gup.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,8 +1238,6 @@ int __mm_populate(unsigned long start, unsigned long len, int ignore_errors)
12381238
int locked = 0;
12391239
long ret = 0;
12401240

1241-
VM_BUG_ON(start & ~PAGE_MASK);
1242-
VM_BUG_ON(len != PAGE_ALIGN(len));
12431241
end = start + len;
12441242

12451243
for (nstart = start; nstart < end; nstart = nend) {

mm/mmap.c

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
186186
return next;
187187
}
188188

189-
static int do_brk(unsigned long addr, unsigned long len, struct list_head *uf);
190-
189+
static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags,
190+
struct list_head *uf);
191191
SYSCALL_DEFINE1(brk, unsigned long, brk)
192192
{
193193
unsigned long retval;
@@ -245,7 +245,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
245245
goto out;
246246

247247
/* Ok, looks good - let it rip. */
248-
if (do_brk(oldbrk, newbrk-oldbrk, &uf) < 0)
248+
if (do_brk_flags(oldbrk, newbrk-oldbrk, 0, &uf) < 0)
249249
goto out;
250250

251251
set_brk:
@@ -2929,21 +2929,14 @@ static inline void verify_mm_writelocked(struct mm_struct *mm)
29292929
* anonymous maps. eventually we may be able to do some
29302930
* brk-specific accounting here.
29312931
*/
2932-
static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags, struct list_head *uf)
2932+
static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long flags, struct list_head *uf)
29332933
{
29342934
struct mm_struct *mm = current->mm;
29352935
struct vm_area_struct *vma, *prev;
2936-
unsigned long len;
29372936
struct rb_node **rb_link, *rb_parent;
29382937
pgoff_t pgoff = addr >> PAGE_SHIFT;
29392938
int error;
29402939

2941-
len = PAGE_ALIGN(request);
2942-
if (len < request)
2943-
return -ENOMEM;
2944-
if (!len)
2945-
return 0;
2946-
29472940
/* Until we need other flags, refuse anything except VM_EXEC. */
29482941
if ((flags & (~VM_EXEC)) != 0)
29492942
return -EINVAL;
@@ -3015,18 +3008,20 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
30153008
return 0;
30163009
}
30173010

3018-
static int do_brk(unsigned long addr, unsigned long len, struct list_head *uf)
3019-
{
3020-
return do_brk_flags(addr, len, 0, uf);
3021-
}
3022-
3023-
int vm_brk_flags(unsigned long addr, unsigned long len, unsigned long flags)
3011+
int vm_brk_flags(unsigned long addr, unsigned long request, unsigned long flags)
30243012
{
30253013
struct mm_struct *mm = current->mm;
3014+
unsigned long len;
30263015
int ret;
30273016
bool populate;
30283017
LIST_HEAD(uf);
30293018

3019+
len = PAGE_ALIGN(request);
3020+
if (len < request)
3021+
return -ENOMEM;
3022+
if (!len)
3023+
return 0;
3024+
30303025
if (down_write_killable(&mm->mmap_sem))
30313026
return -EINTR;
30323027

0 commit comments

Comments
 (0)