From: Oscar Salvador <osalvador@techadventures.net>
To: Michal Hocko <mhocko@kernel.org>
Cc: Zi Yan <zi.yan@cs.rutgers.edu>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
syzbot <syzbot+5dcb560fe12aa5091c06@syzkaller.appspotmail.com>,
akpm@linux-foundation.org, aneesh.kumar@linux.vnet.ibm.com,
dan.j.williams@intel.com, kirill.shutemov@linux.intel.com,
linux-mm@kvack.org, mst@redhat.com,
syzkaller-bugs@googlegroups.com, viro@zeniv.linux.org.uk,
ying.huang@intel.com
Subject: Re: kernel BUG at mm/gup.c:LINE!
Date: Wed, 4 Jul 2018 17:15:29 +0200 [thread overview]
Message-ID: <20180704151529.GA23317@techadventures.net> (raw)
In-Reply-To: <20180704121107.GL22503@dhcp22.suse.cz>
>
> Not really. vm_brk_flags does call mm_populate for mlocked brk which is
> the case for mlockall. I do not see any len sanitization in that path.
> Well do_brk_flags does the roundup. I think we should simply remove the
> bug on and round up there. mm_populate is an internal API and we should
> trust our callers.
>
> Anyway, the minimum fix seems to be the following (untested):
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 9859cd4e19b9..56ad19cf2aea 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -186,8 +186,8 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
> return next;
> }
>
> -static int do_brk(unsigned long addr, unsigned long len, struct list_head *uf);
> -
> +static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags,
> + struct list_head *uf);
> SYSCALL_DEFINE1(brk, unsigned long, brk)
> {
> unsigned long retval;
> @@ -245,7 +245,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
> goto out;
>
> /* Ok, looks good - let it rip. */
> - if (do_brk(oldbrk, newbrk-oldbrk, &uf) < 0)
> + if (do_brk_flags(oldbrk, newbrk-oldbrk, 0, &uf) < 0)
> goto out;
>
> set_brk:
> @@ -2939,12 +2939,6 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
> pgoff_t pgoff = addr >> PAGE_SHIFT;
> int error;
>
> - len = PAGE_ALIGN(request);
> - if (len < request)
> - return -ENOMEM;
> - if (!len)
> - return 0;
> -
> /* Until we need other flags, refuse anything except VM_EXEC. */
> if ((flags & (~VM_EXEC)) != 0)
> return -EINVAL;
> @@ -3016,18 +3010,20 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
> return 0;
> }
>
> -static int do_brk(unsigned long addr, unsigned long len, struct list_head *uf)
> -{
> - return do_brk_flags(addr, len, 0, uf);
> -}
> -
> -int vm_brk_flags(unsigned long addr, unsigned long len, unsigned long flags)
> +int vm_brk_flags(unsigned long addr, unsigned long request, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> + unsigned long len;
> int ret;
> bool populate;
> LIST_HEAD(uf);
>
> + len = PAGE_ALIGN(request);
> + if (len < request)
> + return -ENOMEM;
> + if (!len)
> + return 0;
> +
> if (down_write_killable(&mm->mmap_sem))
> return -EINTR;
I gave this patch a try but the system doesn't boot.
Unfortunately, I don't have the stacktrace on hand, but I'll get back to it tomorrow.
Anyway, I just gave it a try, and making sure that bss gets page aligned seems to
"fix" the issue (at the process doesn't hang anymore):
- bss = eppnt->p_memsz + eppnt->p_vaddr;
+ bss = ELF_PAGESTART(eppnt->p_memsz + eppnt->p_vaddr);
if (bss > len) {
error = vm_brk(len, bss - len);
Although I'm not sure about the correctness of this.
--
Oscar Salvador
SUSE L3
next prev parent reply other threads:[~2018-07-04 15:15 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-04 4:19 kernel BUG at mm/gup.c:LINE! syzbot
2018-07-04 10:01 ` Tetsuo Handa
2018-07-04 11:17 ` Michal Hocko
2018-07-04 11:48 ` Zi Yan
2018-07-04 12:11 ` Michal Hocko
2018-07-04 15:15 ` Oscar Salvador [this message]
2018-07-05 0:35 ` Tetsuo Handa
2018-07-05 7:18 ` Oscar Salvador
2018-07-05 11:40 ` Oscar Salvador
2018-07-05 6:44 ` Michal Hocko
2018-07-05 7:18 ` Oscar Salvador
2018-07-05 12:30 ` Oscar Salvador
2018-07-05 13:40 ` Tetsuo Handa
2018-07-06 5:35 ` Michal Hocko
2018-07-06 7:40 ` Oscar Salvador
2018-07-06 7:50 ` [PATCH] mm: do not bug_on on incorrect lenght in __mm_populate kbuild test robot
2018-07-06 8:23 ` Oscar Salvador
2018-07-06 9:02 ` Michal Hocko
2018-07-04 12:12 ` kernel BUG at mm/gup.c:LINE! Oscar Salvador
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180704151529.GA23317@techadventures.net \
--to=osalvador@techadventures.net \
--cc=akpm@linux-foundation.org \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=dan.j.williams@intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=mst@redhat.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=syzbot+5dcb560fe12aa5091c06@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=viro@zeniv.linux.org.uk \
--cc=ying.huang@intel.com \
--cc=zi.yan@cs.rutgers.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.