linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Michal Hocko <mhocko@kernel.org>
Cc: bugzilla-daemon@bugzilla.kernel.org, linux-mm@kvack.org,
	blurbdust@gmail.com
Subject: Re: [Bug 199037] New: Kernel bug at mm/hugetlb.c:741
Date: Tue, 6 Mar 2018 20:19:44 -0800	[thread overview]
Message-ID: <f91575ec-d158-8876-0096-63a19f0289e0@oracle.com> (raw)
In-Reply-To: <ecc197fa-ae01-8be8-55ec-e82eb1050f57@oracle.com>

On 03/06/2018 04:31 PM, Mike Kravetz wrote:
> On 03/06/2018 01:46 PM, Mike Kravetz wrote:
>> On 03/06/2018 01:31 PM, Andrew Morton wrote:
>>>
>>> That's VM_BUG_ON(resv_map->adds_in_progress) in resv_map_release().
>>>
>>> Do you know if earlier kernel versions are affected?
>>>
>>> It looks quite bisectable.  Does the crash happen every time the test
>>> program is run?
>>
>> I'll take a look.  There was a previous bug in this area:
>> ff8c0c53: mm/hugetlb.c: don't call region_abort if region_chg fails
> 
> This is similar to the issue addressed in 045c7a3f ("fix offset overflow
> in hugetlbfs mmap").  The problem here is that the pgoff argument passed
> to remap_file_pages() is 0x20000000000000.  In the process of converting
> this to a page offset and putting it in vm_pgoff, and then converting back
> to bytes to compute mapping length we end up with 0.  We ultimately end
> up passing (from,to) page offsets into hugetlbfs where from is greater
> than to. :( This confuses the heck out the the huge page reservation code
> as the 'negative' range looks like an error and we never complete the
> reservation process and leave the 'adds_in_progress'.
> 
> This issue has existed for a long time.  The VM_BUG_ON just happens to
> catch the situation which was previously not reported or had some other
> side effect.  Commit 045c7a3f tried to catch these overflow issues when
> converting types, but obviously missed this one.  I can easily add a test
> for this specific value/condition, but want to think about it a little
> more and see if there is a better way to catch all of these.

Well, I instrumented hugetlbfs_file_mmap when called via the remap_file_pages
system call path.  Upon entry, vma->vm_pgoff is 0x20000000000000 which is
the same as the value of the argument pgoff passed to the system call.
vm_pgoff really should be a page offset (i.e. 0x20000000000000 >> PAGE_SHIFT).
So, there is also an issue earlier in the remap_file_pages system call
sequence.

For mmap(), there are architecture specific system call entry points that
do the 'offset >> PAGE_SHIFT' before passing on the value to arch independent
routines.  For remap_file_pages, it looks like sparc is the only arch which
has such a routine.  I know remap_file_pages is deprecated, but could it
really be broken that badly on all architectures but sparc?  Perhaps nobody
really uses it?

To fix, we could add arch specific entry points for all architectures.  But,
that seems like a bunch of effort for a system call that perhaps nobody is
using.  The other option is to remove the sparc entry point, and do the
'pgoff >> PAGE_SHIFT' in the arch independent code.

Thoughts?
-- 
Mike Kravetz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2018-03-07  4:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-199037-27@https.bugzilla.kernel.org/>
2018-03-06 21:31 ` Andrew Morton
2018-03-06 21:46   ` Mike Kravetz
2018-03-07  0:31     ` Mike Kravetz
2018-03-07  2:49       ` Nic Losby
2018-03-07  4:19       ` Mike Kravetz [this message]
2018-03-07 16:39         ` Mike Kravetz
2018-03-07 23:59   ` [PATCH] hugetlbfs: check for pgoff value overflow Mike Kravetz
2018-03-08  0:57     ` Nic Losby
2018-03-08  1:35     ` Yisheng Xie
2018-03-08  4:25       ` Mike Kravetz
2018-03-08 21:03         ` Mike Kravetz
2018-03-08 21:05   ` [PATCH v2] " Mike Kravetz
2018-03-08 22:15     ` Andrew Morton
2018-03-08 23:37       ` Mike Kravetz
2018-03-08 23:53         ` Andrew Morton
2018-03-09  0:27   ` [PATCH v3] " Mike Kravetz
2018-03-09  1:05     ` Andrew Morton
2018-03-16 10:17     ` Michal Hocko
2018-03-16 16:19       ` Mike Kravetz
2018-03-16 16:53         ` Michal Hocko

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=f91575ec-d158-8876-0096-63a19f0289e0@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=blurbdust@gmail.com \
    --cc=bugzilla-daemon@bugzilla.kernel.org \
    --cc=kirill@shutemov.name \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox