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+55aFy23NFG6Vw_FRXSALcCeOXF=48iS4s2rDc_0dAONbOETw@mail.gmail.com>
Date:	Tue, 4 Feb 2014 19:37:43 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>,
	Eric Paris <eparis@...isplace.org>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	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 5:10 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
>
> Umm...  Interactions with aushit might be interesting.

Freudian slip or intentional? :-)

> It hooks into getname() and putname(); I'm not up to doing analysis
> right now [...]

Right you are. I was actually aware of that, but grepping for things
it all looked fine. But I got confused by all the insane audit
wrappers, and you're right, it needs some massaging for audit
handling.

And that audit code really is aushit. I think I found a bug in it
while just scanning it: if audit_alloc_name() fails, the filename will
never be added to the audit lists, and name_count will never be
incremented. But then when we call audit_putname it won't actually put
the name, so it all just leaks - and if you have AUDIT_DEBUG enabled
you'd eventually see an error.

I wonder if we could get rid of some of that crap, and make the audit
code use dentry_path() instead of trying to save off pathnames like
that. But I don't know what the audit code actually *uses* the
pathnames for, so what do I know.

Eric? Can you please explain?

Also, here's a slightly updated patch. The change is that:
 - getname_kernel() will now clear 'filename->aname'
 - cleared 'aname' for regular getname too before calling
audit_getname(), so that if that one fails, it will be NULL.
 - audit_putname() will consider a NULL aname to be the same as not
being in audit context, and just do a final_putname() on it.

That should fix the audit filename leak too, afaik.

Eric, please take a look. As well as explain the audit name thing if possible.

                Linus

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