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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
