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: <CA+55aFyTC1UGvRgH5vASh-C3M+GqHPMNGsUq3Q1MuynzkiSaEA@mail.gmail.com>
Date:	Tue, 4 Feb 2014 16:57:31 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Steven Rostedt <rostedt@...dmis.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, Feb 4, 2014 at 3:42 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
>
> New version that moves all the ugliness into a static inline helper
> function.

Ok, that's better, but I really think we should just use "getname()"
and "putname()".

That's what the path that *matters* already does (ie the normal
execve() system call), so let's just make all the random cases do the
same thing.

That requires a "getname_kernel()" to create a "struct filename *"
from a kernel string, but hey, that's simple enough.

NOTE! This means that "bprm->filename" is no longer a string: it's a
"struct filename *", so if you want the string, you do
"filename->name".

This actually cleans the normal paths up - look how "open_exec()" used
to create a dummy

        struct filename tmp = { .name = name };

on the stack because do_filp_open() wants a 'struct filename' pointer.
I leave that for the external callers (that use it for the
interpreter), but for the main path that actually just goes away,
because now we have that "struct filename *" natively.

It does add code to the special kernel-execve paths, but moving the
"handle errors from getname()" code into do_execve(), even that is
really trivial, eg:

  -       return do_execve(init_filename,
  +       return do_execve(getname_kernel(init_filename),

NOTE NOTE NOTE. This is untested, but it looks fine. If I missed
something, the compiler should warn about bad types. I didn't my the
bprm_flat changes, for example, because that's a no-MMU-only file. And
I might have missed something that didn't match my grep patterns, for
example.

How does this look?

               Linus

View attachment "patch.diff" of type "text/plain" (16836 bytes)