| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Aug 2014 15:44:39 +0100
From: Mel Gorman <mgorman@...e.de>
To: Hugh Dickins <hughd@...gle.com>
Cc: Sasha Levin <sasha.levin@...cle.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Dave Jones <davej@...hat.com>,
LKML <linux-kernel@...r.kernel.org>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Rik van Riel <riel@...hat.com>,
Johannes Weiner <hannes@...xchg.org>,
Cyrill Gorcunov <gorcunov@...il.com>
Subject: Re: mm: BUG in unmap_page_range
On Mon, Aug 04, 2014 at 04:40:38AM -0700, Hugh Dickins wrote:
> On Sat, 2 Aug 2014, Sasha Levin wrote:
>
> > Hi all,
> >
> > While fuzzing with trinity inside a KVM tools guest running the latest -next
> > kernel, I've stumbled on the following spew:
> >
> > [ 2957.087977] BUG: unable to handle kernel paging request at ffffea0003480008
> > [ 2957.088008] IP: unmap_page_range (mm/memory.c:1132 mm/memory.c:1256 mm/memory.c:1277 mm/memory.c:1301)
> > [ 2957.088024] PGD 7fffc6067 PUD 7fffc5067 PMD 0
> > [ 2957.088041] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > [ 2957.088087] Dumping ftrace buffer:
> > [ 2957.088266] (ftrace buffer empty)
> > [ 2957.088279] Modules linked in:
> > [ 2957.088293] CPU: 2 PID: 15417 Comm: trinity-c200 Not tainted 3.16.0-rc7-next-20140801-sasha-00047-gd6ce559 #990
> > [ 2957.088301] task: ffff8807a8c50000 ti: ffff880739fb4000 task.ti: ffff880739fb4000
> > [ 2957.088320] RIP: unmap_page_range (mm/memory.c:1132 mm/memory.c:1256 mm/memory.c:1277 mm/memory.c:1301)
> > [ 2957.088328] RSP: 0018:ffff880739fb7c58 EFLAGS: 00010246
> > [ 2957.088336] RAX: 0000000000000000 RBX: ffff880eb2bdbed8 RCX: dfff971b42800000
> > [ 2957.088343] RDX: 1ffff100e73f6fc4 RSI: 00007f00e85db000 RDI: ffffea0003480008
> > [ 2957.088350] RBP: ffff880739fb7d58 R08: 0000000000000001 R09: 0000000000b6e000
> > [ 2957.088357] R10: 0000000000000000 R11: 0000000000000001 R12: ffffea0003480000
> > [ 2957.088365] R13: 00000000d2000700 R14: 00007f00e85dc000 R15: 00007f00e85db000
> > [ 2957.088374] FS: 00007f00e85d8700(0000) GS:ffff88177fa00000(0000) knlGS:0000000000000000
> > [ 2957.088381] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 2957.088387] CR2: ffffea0003480008 CR3: 00000007a802a000 CR4: 00000000000006a0
> > [ 2957.088406] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 2957.088413] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> > [ 2957.088416] Stack:
> > [ 2957.088432] ffff88171726d570 0000000000000010 0000000000000008 00000000d2000730
> > [ 2957.088450] 0000000019d00250 00007f00e85dc000 ffff880f9d311900 ffff880739fb7e20
> > [ 2957.088466] ffff8807a8c507a0 ffff8807a8c50000 ffff8807a75fe000 ffff8807ceaa7a10
> > [ 2957.088469] Call Trace:
> > [ 2957.088490] unmap_single_vma (mm/memory.c:1348)
> > [ 2957.088505] unmap_vmas (mm/memory.c:1375 (discriminator 3))
> > [ 2957.088520] unmap_region (mm/mmap.c:2386 (discriminator 4))
> > [ 2957.088542] ? vma_rb_erase (mm/mmap.c:454 include/linux/rbtree_augmented.h:219 include/linux/rbtree_augmented.h:227 mm/mmap.c:493)
> > [ 2957.088559] ? vmacache_update (mm/vmacache.c:61)
> > [ 2957.088572] do_munmap (mm/mmap.c:2581)
> > [ 2957.088583] vm_munmap (mm/mmap.c:2596)
> > [ 2957.088595] SyS_munmap (mm/mmap.c:2601)
> > [ 2957.088616] tracesys (arch/x86/kernel/entry_64.S:541)
> > [ 2957.088770] Code: ff ff e8 f9 5f 07 00 48 8b 45 90 80 48 18 01 4d 85 e4 0f 84 8b fe ff ff 45 84 ed 0f 85 fc 03 00 00 49 8d 7c 24 08 e8 b5 67 07 00 <41> f6 44 24 08 01 0f 84 29 02 00 00 83 6d c8 01 4c 89 e7 e8 bd
> > All code
> > ========
> > 0: ff (bad)
> > 1: ff e8 ljmpq *<internal disassembler error>
> > 3: f9 stc
> > 4: 5f pop %rdi
> > 5: 07 (bad)
> > 6: 00 48 8b add %cl,-0x75(%rax)
> > 9: 45 90 rex.RB xchg %eax,%r8d
> > b: 80 48 18 01 orb $0x1,0x18(%rax)
> > f: 4d 85 e4 test %r12,%r12
> > 12: 0f 84 8b fe ff ff je 0xfffffffffffffea3
> > 18: 45 84 ed test %r13b,%r13b
> > 1b: 0f 85 fc 03 00 00 jne 0x41d
> > 21: 49 8d 7c 24 08 lea 0x8(%r12),%rdi
> > 26: e8 b5 67 07 00 callq 0x767e0
> > 2b:* 41 f6 44 24 08 01 testb $0x1,0x8(%r12) <-- trapping instruction
> > 31: 0f 84 29 02 00 00 je 0x260
> > 37: 83 6d c8 01 subl $0x1,-0x38(%rbp)
> > 3b: 4c 89 e7 mov %r12,%rdi
> > 3e: e8 .byte 0xe8
> > 3f: bd .byte 0xbd
>
> This differs in which functions got inlined (unmap_page_range showing up
> in place of zap_pte_range), but this is the same "if (PageAnon(page))"
> that Sasha reported in the "hang in shmem_fallocate" thread on June 26th.
>
> I can see what it is now, and here is most of a patch (which I don't
> expect to satisfy Trinity yet); at this point I think I had better
> hand it over to Mel, to complete or to discard.
>
> [INCOMPLETE PATCH] x86,mm: fix pte_special versus pte_numa
>
> Sasha Levin has shown oopses on ffffea0003480048 and ffffea0003480008
> at mm/memory.c:1132, running Trinity on different 3.16-rc-next kernels:
> where zap_pte_range() checks page->mapping to see if PageAnon(page).
>
> Those addresses fit struct pages for pfns d2001 and d2000, and in each
> dump a register or a stack slot showed d2001730 or d2000730: pte flags
> 0x730 are PCD ACCESSED PROTNONE SPECIAL IOMAP; and Sasha's e820 map has
> a hole between cfffffff and 100000000, which would need special access.
>
> Commit c46a7c817e66 ("x86: define _PAGE_NUMA by reusing software bits on
> the PMD and PTE levels") has broken vm_normal_page(): a PROTNONE SPECIAL
> pte no longer passes the pte_special() test, so zap_pte_range() goes on
> to try to access a non-existent struct page.
>
:(
> Fix this by refining pte_special() (SPECIAL with PRESENT or PROTNONE)
> to complement pte_numa() (SPECIAL with neither PRESENT nor PROTNONE).
>
> It's unclear why c46a7c817e66 added pte_numa() test to vm_normal_page(),
> and moved its is_zero_pfn() test from slow to fast path: I suspect both
> were papering over PROT_NONE issues seen with inadequate pte_special().
> Revert vm_normal_page() to how it was before, relying on pte_special().
>
Rather than answering directly I updated your changelog
Fix this by refining pte_special() (SPECIAL with PRESENT or PROTNONE)
to complement pte_numa() (SPECIAL with neither PRESENT nor PROTNONE).
A hint that this was a problem was that c46a7c817e66 added pte_numa()
test to vm_normal_page(), and moved its is_zero_pfn() test from slow to
fast path: This was papering over a pte_special() snag when the zero
page was encountered during zap. This patch reverts vm_normal_page()
to how it was before, relying on pte_special().
> I find it confusing, that the only example of ARCH_USES_NUMA_PROT_NONE
> no longer uses PROTNONE for NUMA, but SPECIAL instead: update the
> asm-generic comment a little, but that config option remains unhelpful.
>
ARCH_USES_NUMA_PROT_NONE should have been sent to the farm at the same time
as that patch and by rights unified with the powerpc helpers. With the new
_PAGE_NUMA bit, there is no reason they should have different implementations
of pte_numa and related functions. Unfortunately unifying them is a little
problematic due to differences in fundamental types. It could be done with
#defines but I'm attaching a preliminary prototype to illustrate the issue.
> But more seriously, I think this patch is incomplete: aren't there
> other places which need to be handling PROTNONE along with PRESENT?
> For example, pte_mknuma() clears _PAGE_PRESENT and sets _PAGE_NUMA,
> but on a PROT_NONE area, I think that will now make it pte_special()?
> So it ought to clear _PAGE_PROTNONE too. Or maybe we can never
> pte_mknuma() on a PROT_NONE area - there would be no point?
>
We are depending on the fact that inaccessible VMAs are skipped by the
NUMA hinting scanner.
> Around here I began to wonder if it was just a mistake to have deserted
> the PROTNONE for NUMA model: I know Linus had a strong reaction against
> it, and I've never delved into its drawbacks myself; but bringing yet
> another (SPECIAL) flag into the game is not an obvious improvement.
> Should we just revert c46a7c817e66, or would that be a mistake?
>
It's replacing one type of complexity with another. The downside is that
_PAGE_NUMA == _PAGE_PROTNONE puts subtle traps all over the core for
powerpc to fall foul of.
I'm attaching a preliminary pair of patches. The first which deals with
ARCH_USES_NUMA_PROT_NONE and the second which is yours with a revised
changelog. I'm adding Aneesh to the cc to look at the powerpc portion of
the first patch.
--
Mel Gorman
SUSE Labs
View attachment "0001" of type "text/plain" (7777 bytes)
View attachment "0002" of type "text/plain" (3947 bytes)
Powered by blists - more mailing lists