linux-kernel - Re: [RFC][PATCH] exec: Fix use after free of tracepoint trace_sched_process

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]

Message-ID: <20140204182852.43ffa442@gandalf.local.home>
Date:	Tue, 4 Feb 2014 18:28:52 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	David Smith <dsmith@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Igor Zhbanov <i.zhbanov@...sung.com>,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [RFC][PATCH] exec: Fix use after free of tracepoint
 trace_sched_process_exec

On Tue, 4 Feb 2014 12:18:53 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> That's too ugly to live.

New patch. Not as ugly. Well, I think this one lacks ugly enough to be
worth living for.

It's dependent on another patch that adds a helper function for
tracepoints, that allows users to implicitly use static_key of a
tracepoint to see if it is enabled or not. Basically, it's:

#define tracepoint_enabled(name) \
	static_key_false(&__tracepoint_##name.key)

This uses the same key that the tracepoint has when it is enabled. It
may be enabled before or after the tracepoint is, but in cases like
these, it doesn't really matter.

At least this patch keeps the ugliness with the code. I could even
encapsulate that in a static inline function to remove the ugliness out
of exec_binprm().

Note, this still requires some comments added to the code.

Butt-ugly-by: Steven Rostedt <rostedt@...dmis.org>

diff --git a/fs/exec.c b/fs/exec.c
index e1529b4..f7902aef 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1421,7 +1421,22 @@ static int exec_binprm(struct linux_binprm *bprm)
 	ret = search_binary_handler(bprm);
 	if (ret >= 0) {
 		audit_bprm(bprm);
-		trace_sched_process_exec(current, old_pid, bprm);
+		if (tracepoint_enabled(sched_process_exec)) {
+			char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
+			char *pathname;
+
+			if (tmp)
+				pathname = dentry_path_raw(bprm->file->f_dentry,
+							   tmp, PAGE_SIZE);
+			else
+				pathname = ERR_PTR(-ENOMEM);
+			if (IS_ERR(pathname))
+				trace_sched_process_exec(current, old_pid,
+							 "//error-no-mem");
+			else
+				trace_sched_process_exec(current, old_pid, pathname);
+			free_page((unsigned long)tmp);
+		}
 		ptrace_event(PTRACE_EVENT_EXEC, old_vpid);
 		proc_exec_connector(current);
 	}
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 67e1bbf..520ba9a 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -282,18 +282,18 @@ TRACE_EVENT(sched_process_fork,
 TRACE_EVENT(sched_process_exec,
 
 	TP_PROTO(struct task_struct *p, pid_t old_pid,
-		 struct linux_binprm *bprm),
+		 const char *pathname),
 
-	TP_ARGS(p, old_pid, bprm),
+	TP_ARGS(p, old_pid, pathname),
 
 	TP_STRUCT__entry(
-		__string(	filename,	bprm->filename	)
+		__string(	filename,	pathname	)
 		__field(	pid_t,		pid		)
 		__field(	pid_t,		old_pid		)
 	),
 
 	TP_fast_assign(
-		__assign_str(filename, bprm->filename);
+		__assign_str(filename, pathname);
 		__entry->pid		= p->pid;
 		__entry->old_pid	= old_pid;
 	),
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/