On Fri, Jul 04, 2014 at 10:55:13AM -0400, Sasha Levin wrote:
> On 07/03/2014 05:37 PM, David Rientjes wrote:
> > On Wed, 2 Jul 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:
> >>>
> >>> [ 3569.869749] general protection fault: 0000 [#1] PREEMPT SMP 
> >>> DEBUG_PAGEALLOC
> >>> [ 3569.869769] Dumping ftrace buffer:
> >>> [ 3569.869879]    (ftrace buffer empty)
> >>> [ 3569.869894] Modules linked in:
> >>> [ 3569.869900] CPU: 7 PID: 10239 Comm: trinity-c86 Tainted: G        W    
> >>>   3.16.0-rc3-next-20140701-sasha-00023-g4eb2544-dirty #759
> >>> [ 3569.869906] task: ffff88039e873000 ti: ffff880393f8c000 task.ti: 
> >>> ffff880393f8c000
> >>> [ 3569.869932] RIP: show_mountinfo (fs/proc_namespace.c:127)

So that would be this line then:

static int show_mountinfo(struct seq_file *m, struct vfsmount *mnt)
{
        struct proc_mounts *p = proc_mounts(m);
        struct mount *r = real_mount(mnt);
        struct super_block *sb = mnt->mnt_sb;
        struct path mnt_path = { .dentry = mnt->mnt_root, .mnt = mnt }; <---

if I understand the output correctly?

> >>> All code
> >>> ========
> >>>    0:     00 00                   add    %al,(%rax)
> >>>    2:     00 00                   add    %al,(%rax)
> >>>    4:     66 66 66 66 90          data32 data32 data32 xchg %ax,%ax
> >>>    9:     55                      push   %rbp
> >>>    a:     48 89 e5                mov    %rsp,%rbp
> >>>    d:     48 83 ec 50             sub    $0x50,%rsp
> >>>   11:     48 89 5d d8             mov    %rbx,-0x28(%rbp)
> >>>   15:     48 89 f3                mov    %rsi,%rbx
> >>>   18:     4c 89 65 e0             mov    %r12,-0x20(%rbp)
> >>>   1c:     49 89 fc                mov    %rdi,%r12
> >>>   1f:     4c 89 6d e8             mov    %r13,-0x18(%rbp)
> >>>   23:     4c 89 75 f0             mov    %r14,-0x10(%rbp)
> >>>   27:     4c 89 7d f8             mov    %r15,-0x8(%rbp)
> >>>   2b:*    48 8b 06                mov    (%rsi),%rax              <-- 
> >>> trapping instruction
> >>>   2e:     48 89 75 b0             mov    %rsi,-0x50(%rbp)
> >>>   32:     4c 8b 76 08             mov    0x8(%rsi),%r14
> >>>   36:     8b 96 ec 00 00 00       mov    0xec(%rsi),%edx
> >>>   3c:     48 89 45 b8             mov    %rax,-0x48(%rbp)

Does that fit to this asm code? Sorry.. I'm not very familar with x86 asm.

> > Does this now reproduce on Linus's tree?  If so, does reverting commit
> > 058504edd026 ("fs/seq_file: fallback to vmalloc allocation") prevent this 
> > issue?
> > 
> > This is a use-after-free since the poison value is 0x6b and I'm presuming 
> > that your /proc/self/mountinfo may be larger than PAGE_SIZE in your 
> > testing environment.
> 
> Good call, reverting that patch made both issues go away.

Ok, does that mean you have a test case so you can reproduce the crashes?
Just wondering, since trinity is usually quite random in what it does.

I tried to reproduce a crash on one of my systems by enforcing several
seq_files to grow beyond PAGE_SIZE, but couldn't reproduce anything yet.

Could you try the patch below please? It basically reverts my patch and
just leaves the kfree->kvfree conversion in. This is just a shot in the
dark, since I can't make any sense of this ...yet :)

diff --git a/fs/seq_file.c b/fs/seq_file.c
index 3857b720cb1b..c1cf494cc238 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -34,12 +34,7 @@ static void seq_set_overflow(struct seq_file *m)
 
 static void *seq_buf_alloc(unsigned long size)
 {
-       void *buf;
-
-       buf = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
-       if (!buf && size > PAGE_SIZE)
-               buf = vmalloc(size);
-       return buf;
+       return kmalloc(size, GFP_KERNEL);
 }
 
 /**

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to