linux-kernel - Re: [for-next][PATCH 4/8] ftrace: Reset fgd->hash in ftrace_graph

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 3 Feb 2017 23:49:38 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [for-next][PATCH 4/8] ftrace: Reset fgd->hash in ftrace_graph_write()

On Fri, Feb 3, 2017 at 10:40 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@...dmis.org>
>
> fgd->hash is saved and then freed, but is never reset to either
> ftrace_graph_hash nor ftrace_graph_notrace_hash. But if multiple reads are
> performed, then the freed hash could be accessed again.

Argh, right.  Btw did you mean multiple "write" not "read", no?

Thanks,
Namhyung


>
>  # cd /sys/kernel/debug/tracing
>  # head -1000 available_filter_functions > /tmp/funcs
>  # cat /tmp/funcs > set_graph_function
>
> Causes:
>
>  general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
>  Modules linked in:  [...]
>  CPU: 2 PID: 1337 Comm: cat Not tainted 4.10.0-rc2-test-00010-g6b052e9 #32
>  Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012
>  task: ffff880113a12200 task.stack: ffffc90001940000
>  RIP: 0010:free_ftrace_hash+0x7c/0x160
>  RSP: 0018:ffffc90001943db0 EFLAGS: 00010246
>  RAX: 6b6b6b6b6b6b6b6b RBX: 6b6b6b6b6b6b6b6b RCX: 6b6b6b6b6b6b6b6b
>  RDX: 0000000000000002 RSI: 0000000000000001 RDI: ffff8800ce1e1d40
>  RBP: ffff8800ce1e1d50 R08: 0000000000000000 R09: 0000000000006400
>  R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
>  R13: ffff8800ce1e1d40 R14: 0000000000004000 R15: 0000000000000001
>  FS:  00007f9408a07740(0000) GS:ffff88011e500000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000aee1f0 CR3: 0000000116bb4000 CR4: 00000000001406e0
>  Call Trace:
>   ? ftrace_graph_write+0x150/0x190
>   ? __vfs_write+0x1f6/0x210
>   ? __audit_syscall_entry+0x17f/0x200
>   ? rw_verify_area+0xdb/0x210
>   ? _cond_resched+0x2b/0x50
>   ? __sb_start_write+0xb4/0x130
>   ? vfs_write+0x1c8/0x330
>   ? SyS_write+0x62/0xf0
>   ? do_syscall_64+0xa3/0x1b0
>   ? entry_SYSCALL64_slow_path+0x25/0x25
>  Code: 01 48 85 db 0f 84 92 00 00 00 b8 01 00 00 00 d3 e0 85 c0 7e 3f 83 e8 01 48 8d 6f 10 45 31 e4 4c 8d 34 c5 08 00 00 00 49 8b 45 08 <4a> 8b 34 20 48 85 f6 74 13 48 8b 1e 48 89 ef e8 20 fa ff ff 48
>  RIP: free_ftrace_hash+0x7c/0x160 RSP: ffffc90001943db0
>  ---[ end trace 999b48216bf4b393 ]---
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> ---
>  kernel/trace/ftrace.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index a9cfc8713198..b7df0dcf8652 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4858,10 +4858,13 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
>                 if (!new_hash)
>                         ret = -ENOMEM;
>
> -               if (fgd->type == GRAPH_FILTER_FUNCTION)
> +               if (fgd->type == GRAPH_FILTER_FUNCTION) {
>                         rcu_assign_pointer(ftrace_graph_hash, new_hash);
> -               else
> +                       fgd->hash = ftrace_graph_hash;
> +               } else {
>                         rcu_assign_pointer(ftrace_graph_notrace_hash, new_hash);
> +                       fgd->hash = ftrace_graph_notrace_hash;
> +               }
>
>                 mutex_unlock(&graph_lock);
>
> --
> 2.10.2
>
>