fs: use after free in /proc/pid/mountinfo

DISCUSSION Sasha Levin 5 participants 16 messages Jul 2, 2014 – Jul 23, 2014

Subsystems (2)

Stable branch VFS

lore

Share & tools

lore

David Rientjes

2014-07-03 21:37 UTC #2 lore raw

Quoted 56 lines · Sasha Levin

Quoted 56 lines

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)
[ 3569.869938] RSP: 0018:ffff880393f8fd08  EFLAGS: 00010282
[ 3569.869943] RAX: ffff880200c6d208 RBX: 6b6b6b6b6b6b6b03 RCX: 0000000000000074
[ 3569.869946] RDX: 0000000000000000 RSI: 6b6b6b6b6b6b6b03 RDI: ffff880200c6d208
[ 3569.869953] RBP: ffff880393f8fd58 R08: 0000000000000001 R09: ffff88039e873d38
[ 3569.869957] R10: 0000000000000001 R11: 0000000000000000 R12: ffff880200c6d208
[ 3569.869962] R13: 6b6b6b6b6b6b6b6b R14: ffff880393f8ff50 R15: ffff880200c6d208
[ 3569.869966] FS:  00007f6f20db9700(0000) GS:ffff8801ece00000(0000) knlGS:0000000000000000
[ 3569.869969] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3569.869974] CR2: 00007f6f20bee000 CR3: 00000003a0775000 CR4: 00000000000006a0
[ 3569.869992] Stack:
[ 3569.870011]  ffff880393f8ff50 ffff880200c6d208 ffff880393f8fd48 ffffffff984e10ea
[ 3569.870025]  ffffffff9532c7fe ffff8802069a3b80 0000000000000065 6b6b6b6b6b6b6b6b
[ 3569.870036]  ffff880393f8ff50 ffff880200c6d208 ffff880393f8fd68 ffffffff9532c756
[ 3569.870038] Call Trace:
[ 3569.870051] ? down_read (kernel/locking/rwsem.c:45 (discriminator 2))
[ 3569.870059] ? m_start (kernel/module.c:3634)
[ 3569.870069] m_show (kernel/module.c:3649)
[ 3569.870076] seq_read (fs/seq_file.c:235)
[ 3569.870081] ? single_open_size (fs/seq_file.c:166)
[ 3569.870092] do_loop_readv_writev (fs/read_write.c:708)
[ 3569.870098] ? single_open_size (fs/seq_file.c:166)
[ 3569.870098] do_readv_writev (fs/read_write.c:840)
[ 3569.870098] ? get_parent_ip (kernel/sched/core.c:2550)
[ 3569.870098] ? mutex_lock_nested (./arch/x86/include/asm/preempt.h:98 kernel/locking/mutex.c:570 kernel/locking/mutex.c:587)
[ 3569.870098] ? __fdget_pos (fs/file.c:714)
[ 3569.870098] ? __fdget_pos (fs/file.c:714)
[ 3569.870098] ? trace_hardirqs_on_caller (kernel/locking/lockdep.c:2557 kernel/locking/lockdep.c:2599)
[ 3569.870098] ? __fget_light (fs/file.c:685)
[ 3569.870098] vfs_readv (fs/read_write.c:866)
[ 3569.870098] SyS_readv (fs/read_write.c:892 fs/read_write.c:884)
[ 3569.870098] tracesys (arch/x86/kernel/entry_64.S:542)
[ 3569.870098] Code: 00 00 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 50 48 89 5d d8 48 89 f3 4c 89 65 e0 49 89 fc 4c 89 6d e8 4c 89 75 f0 4c 89 7d f8 <48> 8b 06 48 89 75 b0 4c 8b 76 08 8b 96 ec 00 00 00 48 89 45 b8
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

Quoted 8 lines

  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 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.

Sasha Levin

author

2014-07-04 14:55 UTC #3 lore raw

Quoted 67 lines · David Rientjes

Quoted 56 lines · Sasha Levin

Quoted 56 lines

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)
[ 3569.869938] RSP: 0018:ffff880393f8fd08  EFLAGS: 00010282
[ 3569.869943] RAX: ffff880200c6d208 RBX: 6b6b6b6b6b6b6b03 RCX: 0000000000000074
[ 3569.869946] RDX: 0000000000000000 RSI: 6b6b6b6b6b6b6b03 RDI: ffff880200c6d208
[ 3569.869953] RBP: ffff880393f8fd58 R08: 0000000000000001 R09: ffff88039e873d38
[ 3569.869957] R10: 0000000000000001 R11: 0000000000000000 R12: ffff880200c6d208
[ 3569.869962] R13: 6b6b6b6b6b6b6b6b R14: ffff880393f8ff50 R15: ffff880200c6d208
[ 3569.869966] FS:  00007f6f20db9700(0000) GS:ffff8801ece00000(0000) knlGS:0000000000000000
[ 3569.869969] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3569.869974] CR2: 00007f6f20bee000 CR3: 00000003a0775000 CR4: 00000000000006a0
[ 3569.869992] Stack:
[ 3569.870011]  ffff880393f8ff50 ffff880200c6d208 ffff880393f8fd48 ffffffff984e10ea
[ 3569.870025]  ffffffff9532c7fe ffff8802069a3b80 0000000000000065 6b6b6b6b6b6b6b6b
[ 3569.870036]  ffff880393f8ff50 ffff880200c6d208 ffff880393f8fd68 ffffffff9532c756
[ 3569.870038] Call Trace:
[ 3569.870051] ? down_read (kernel/locking/rwsem.c:45 (discriminator 2))
[ 3569.870059] ? m_start (kernel/module.c:3634)
[ 3569.870069] m_show (kernel/module.c:3649)
[ 3569.870076] seq_read (fs/seq_file.c:235)
[ 3569.870081] ? single_open_size (fs/seq_file.c:166)
[ 3569.870092] do_loop_readv_writev (fs/read_write.c:708)
[ 3569.870098] ? single_open_size (fs/seq_file.c:166)
[ 3569.870098] do_readv_writev (fs/read_write.c:840)
[ 3569.870098] ? get_parent_ip (kernel/sched/core.c:2550)
[ 3569.870098] ? mutex_lock_nested (./arch/x86/include/asm/preempt.h:98 kernel/locking/mutex.c:570 kernel/locking/mutex.c:587)
[ 3569.870098] ? __fdget_pos (fs/file.c:714)
[ 3569.870098] ? __fdget_pos (fs/file.c:714)
[ 3569.870098] ? trace_hardirqs_on_caller (kernel/locking/lockdep.c:2557 kernel/locking/lockdep.c:2599)
[ 3569.870098] ? __fget_light (fs/file.c:685)
[ 3569.870098] vfs_readv (fs/read_write.c:866)
[ 3569.870098] SyS_readv (fs/read_write.c:892 fs/read_write.c:884)
[ 3569.870098] tracesys (arch/x86/kernel/entry_64.S:542)
[ 3569.870098] Code: 00 00 00 00 66 66 66 66 90 55 48 89 e5 48 83 ec 50 48 89 5d d8 48 89 f3 4c 89 65 e0 49 89 fc 4c 89 6d e8 4c 89 75 f0 4c 89 7d f8 <48> 8b 06 48 89 75 b0 4c 8b 76 08 8b 96 ec 00 00 00 48 89 45 b8
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

Quoted 8 lines

  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)

Quoted 8 lines

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.

Thanks,
Sasha

David Rientjes

2014-07-04 22:35 UTC #4 lore raw

[stable] please do not merge 058504edd026 (was Re: fs: use after free in /proc/pid/mountinfo)

Quoted 11 lines · Sasha Levin

Quoted 8 lines
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.

Thanks for checking, Sasha.

Stable maintainers, please do not merge commit 058504edd026 ("fs/seq_file:
fallback to vmalloc allocation") that is annotated with a cc for [email protected] in Linus's tree into stable kernels.

Although we're at 3.16-rc3, I'm hoping that we can get a fix for the use-after-free in the next couple days before asking for a revert. Sasha confirms[*] this commit causes the bug.

Thanks.

[*] http://marc.info/?l=linux-kernel&m=140448573612154

Greg KH

2014-07-07 23:01 UTC #5 lore raw

Re: [stable] please do not merge 058504edd026 (was Re: fs: use after free in /proc/pid/mountinfo)

Quoted 16 lines · David Rientjes

Quoted 11 lines · Sasha Levin
Quoted 8 lines
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.
Thanks for checking, Sasha.

Quoted 7 lines

Stable maintainers, please do not merge commit 058504edd026 ("fs/seq_file:
fallback to vmalloc allocation") that is annotated with a cc for [email protected] in Linus's tree into stable kernels.
Although we're at 3.16-rc3, I'm hoping that we can get a fix for the use-after-free in the next couple days before asking for a revert. Sasha confirms[*] this commit causes the bug.

I should also drop "Subject: /proc/stat: convert to single_open_size()" from the -stable tree, right?

thanks,

greg k-h

Andrew Morton

2014-07-07 23:06 UTC #6 lore raw

Re: [stable] please do not merge 058504edd026 (was Re: fs: use after free in /proc/pid/mountinfo)

Quoted 21 lines · On Mon, 7 Jul 2014 16:05:42 -0700 Greg KH <[email protected]> wrote:

Quoted 20 lines · David Rientjes
Quoted 11 lines · Sasha Levin
Quoted 8 lines
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.
Thanks for checking, Sasha.
Stable maintainers, please do not merge commit 058504edd026 ("fs/seq_file:
fallback to vmalloc allocation") that is annotated with a cc for [email protected] in Linus's tree into stable kernels.

Quoted 7 lines

Quoted 3 lines
Although we're at 3.16-rc3, I'm hoping that we can get a fix for the use-after-free in the next couple days before asking for a revert. Sasha confirms[*] this commit causes the bug.
I should also drop "Subject: /proc/stat: convert to single_open_size()" from the -stable tree, right?

That would be best.

I can't see how "fs: use after free in /proc/pid/mountinfo" can cause a use-after-free so perhaps the bug lies elsewhere and was hidden by luck (slab buffering or slab rcu-freeing or something). In which case "fs/seq_file: fallback to vmalloc allocation" might be the patch which added the bug.

Greg KH

2014-07-07 23:15 UTC #7 lore raw

Re: [stable] please do not merge 058504edd026 (was Re: fs: use after free in /proc/pid/mountinfo)

Quoted 27 lines · Andrew Morton

Quoted 25 lines · On Mon, 7 Jul 2014 16:05:42 -0700 Greg KH <[email protected]> wrote:
Quoted 23 lines · David Rientjes
Quoted 11 lines · Sasha Levin
Quoted 8 lines
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.
Thanks for checking, Sasha.
Stable maintainers, please do not merge commit 058504edd026 ("fs/seq_file:
fallback to vmalloc allocation") that is annotated with a cc for [email protected] in Linus's tree into stable kernels.
Although we're at 3.16-rc3, I'm hoping that we can get a fix for the use-after-free in the next couple days before asking for a revert. Sasha confirms[*] this commit causes the bug.

Quoted 5 lines

Quoted 3 lines
I should also drop "Subject: /proc/stat: convert to single_open_size()" from the -stable tree, right?
That would be best.

Ok, now dropped, thanks.

greg k-h

Heiko Carstens

2014-07-06 10:04 UTC #8 lore raw

In reply to Sasha Levin

Quoted 8 lines · Sasha Levin

Quoted 7 lines · David Rientjes
Quoted 5 lines · Sasha Levin
Quoted 5 lines
Hi all,
While fuzzing with trinity inside a KVM tools guest running the latest -next kernel I've stumbled on the following spew:

Quoted 7 lines

[ 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?

Quoted 12 lines

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

Quoted 8 lines

  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.

Quoted 9 lines

Quoted 7 lines
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 :)

fs/seq_file.c +1 -6

index 3857b720cb1b..c1cf494cc238 100644
@@ -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);
 }
 
 /**

Heiko Carstens

2014-07-09 14:24 UTC #9 lore raw

[full quote of my mail below]

058504edd026 ("fs/seq_file: fallback to vmalloc allocation") is suspected to cause a crash. Actually I can't reprocude the crash nor would I be able to tell how the commit could cause the crash.
Anyway, I'll be offline for the next 2.5 weeks. So if Sasha could confirm that reverting the patch actually does fix the crash, please revert the commit, unless somebody else can make sense of the report of course.

I'm still wondering how Sasha could reproduce the crash.

Thanks,
Heiko

Quoted 95 lines · Heiko Carstens

Quoted 8 lines · Sasha Levin
Quoted 7 lines · David Rientjes
Quoted 5 lines · Sasha Levin
Quoted 5 lines
Hi all,
While fuzzing with trinity inside a KVM tools guest running the latest -next kernel I've stumbled on the following spew:
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
[ 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?
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
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
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
  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.
Quoted 9 lines
Quoted 7 lines
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 :)
fs/seq_file.c +1 -7
index 3857b720cb1b..c1cf494cc238 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andrew Morton

2014-07-09 20:31 UTC #10 lore raw

Quoted 8 lines · On Wed, 9 Jul 2014 16:24:06 +0200 Heiko Carstens <[email protected]>…

058504edd026 ("fs/seq_file: fallback to vmalloc allocation") is suspected to cause a crash. Actually I can't reprocude the crash nor would I be able to tell how the commit could cause the crash.
Anyway, I'll be offline for the next 2.5 weeks. So if Sasha could confirm that reverting the patch actually does fix the crash, please revert the commit, unless somebody else can make sense of the report of course.
I'm still wondering how Sasha could reproduce the crash.

This really sucks :(

I suppose I'll queue up a revert of 058504edd026 as a for-3.16 bandaid.

Sasha, it would be great if you could delve a bit further into this, see if we can identify a way for others to reproduce the bug.

Did you test that little patchlet which Heiko sent?

fs/seq_file.c +1 -6

@@ -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);
 }
 
 /**

It would also be interesting to try the opposite: always use vmalloc(). That may make it easier to trigger the bug.

Sasha Levin

author

2014-07-09 22:10 UTC #11 lore raw

Quoted 10 lines · Andrew Morton

Quoted 8 lines · On Wed, 9 Jul 2014 16:24:06 +0200 Heiko Carstens <[email protected]>…
Quoted 8 lines
058504edd026 ("fs/seq_file: fallback to vmalloc allocation") is suspected to cause a crash. Actually I can't reprocude the crash nor would I be able to tell how the commit could cause the crash.
Anyway, I'll be offline for the next 2.5 weeks. So if Sasha could confirm that reverting the patch actually does fix the crash, please revert the commit, unless somebody else can make sense of the report of course.
I'm still wondering how Sasha could reproduce the crash.

Quoted 8 lines

This really sucks :(
I suppose I'll queue up a revert of 058504edd026 as a for-3.16 bandaid.
Sasha, it would be great if you could delve a bit further into this, see if we can identify a way for others to reproduce the bug.
Did you test that little patchlet which Heiko sent?

I've been trying, but as there isn't an easy way to reproduce it, breakage elsewhere in -next (there is a long list) is slowing me down. I'll revert back to the -next where I've initially observed this issue and will retry.

Thanks,
Sasha

Andrew Morton

2014-07-09 22:59 UTC #12 lore raw

Quoted 15 lines · On Wed, 09 Jul 2014 18:10:47 -0400 Sasha Levin <[email protected]> wrote:

Quoted 14 lines · Andrew Morton
Quoted 8 lines · On Wed, 9 Jul 2014 16:24:06 +0200 Heiko Carstens <[email protected]>…
Quoted 8 lines
058504edd026 ("fs/seq_file: fallback to vmalloc allocation") is suspected to cause a crash. Actually I can't reprocude the crash nor would I be able to tell how the commit could cause the crash.
Anyway, I'll be offline for the next 2.5 weeks. So if Sasha could confirm that reverting the patch actually does fix the crash, please revert the commit, unless somebody else can make sense of the report of course.
I'm still wondering how Sasha could reproduce the crash.
This really sucks :(
I suppose I'll queue up a revert of 058504edd026 as a for-3.16 bandaid.

Quoted 9 lines

Quoted 4 lines
Sasha, it would be great if you could delve a bit further into this, see if we can identify a way for others to reproduce the bug.
Did you test that little patchlet which Heiko sent?
I've been trying, but as there isn't an easy way to reproduce it, breakage elsewhere in -next (there is a long list) is slowing me down. I'll revert back to the -next where I've initially observed this issue and will retry.

Thanks. The bug should be present in current mainline.

Sasha Levin

author

2014-07-15 13:52 UTC #13 lore raw

Quoted 20 lines · Andrew Morton

Quoted 18 lines · On Wed, 09 Jul 2014 18:10:47 -0400 Sasha Levin <[email protected]> wrote:
Quoted 17 lines · Andrew Morton
Quoted 8 lines · On Wed, 9 Jul 2014 16:24:06 +0200 Heiko Carstens <[email protected]>…
Quoted 8 lines
058504edd026 ("fs/seq_file: fallback to vmalloc allocation") is suspected to cause a crash. Actually I can't reprocude the crash nor would I be able to tell how the commit could cause the crash.
Anyway, I'll be offline for the next 2.5 weeks. So if Sasha could confirm that reverting the patch actually does fix the crash, please revert the commit, unless somebody else can make sense of the report of course.
I'm still wondering how Sasha could reproduce the crash.
This really sucks :(
I suppose I'll queue up a revert of 058504edd026 as a for-3.16 bandaid.
Sasha, it would be great if you could delve a bit further into this, see if we can identify a way for others to reproduce the bug.

Quoted 9 lines

Quoted 6 lines
Quoted 1 line
Did you test that little patchlet which Heiko sent?
I've been trying, but as there isn't an easy way to reproduce it, breakage elsewhere in -next (there is a long list) is slowing me down. I'll revert back to the -next where I've initially observed this issue and will retry.
Thanks. The bug should be present in current mainline.

I've been running mainline for a while, and while I can reproduce a different bug for Hugh, I can't reproduce this one at all. Not sure how to call it now, maybe memory corruption from somewhere else (I did find an unrelated memory corruption couple days ago)?

Thanks,
Sasha

David Rientjes

2014-07-15 23:01 UTC #14 lore raw

Quoted 5 lines · Sasha Levin

I've been running mainline for a while, and while I can reproduce a different bug for Hugh, I can't reproduce this one at all. Not sure how to call it now, maybe memory corruption from somewhere else (I did find an unrelated memory corruption couple days ago)?

I have a very large /proc/pid/mountinfo on one of my busy systems with VM and slub debugging enabled and have not been able to reproduce the issue while reading it in a loop for 24 hours. I would call the patch good on that basis, but it's very strange you'd report this problem a day after the patch was merged and could not reproduce it once it was reverted. (Unless the patch was misapplied locally and reapplying it now worked, but I doubt you originally merged -mm patches into a local tree yourself.)

Andrew Morton

2014-07-23 20:57 UTC #15 lore raw

Quoted 8 lines · On Tue, 15 Jul 2014 16:01:34 -0700 (PDT) David Rientjes <[email protected]>…

Quoted 5 lines · Sasha Levin
I've been running mainline for a while, and while I can reproduce a different bug for Hugh, I can't reproduce this one at all. Not sure how to call it now, maybe memory corruption from somewhere else (I did find an unrelated memory corruption couple days ago)?

Quoted 7 lines

I have a very large /proc/pid/mountinfo on one of my busy systems with VM and slub debugging enabled and have not been able to reproduce the issue while reading it in a loop for 24 hours. I would call the patch good on that basis, but it's very strange you'd report this problem a day after the patch was merged and could not reproduce it once it was reverted. (Unless the patch was misapplied locally and reapplying it now worked, but I doubt you originally merged -mm patches into a local tree yourself.)

I was going to revert "fs/seq_file: fallback to vmalloc allocation" but the idea of doing that is so lame and pathetic that I think I'll not do that. If we have a bug then let's just find and fix it when someone is able to trigger it.