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

Powered by blists - more mailing lists