linux-kernel - Re: [PATCH] Fix for binary

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: <m1iplerglz.fsf@fess.ebiederm.org>
Date:	Sat, 17 Dec 2011 14:14:00 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Michel Lespinasse <walken@...gle.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix for binary_sysctl() memory leak

Andrew Morton <akpm@...ux-foundation.org> writes:

> On Thu, 15 Dec 2011 14:38:58 -0800
> Michel Lespinasse <walken@...gle.com> wrote:
>
>> On Thu, Dec 15, 2011 at 2:19 PM, Andrew Morton
>> <akpm@...ux-foundation.org> wrote:
>> > I think the patch is correct but the description is misleading?
>> >
>> > I see no memory leak here. __Calling __putname() directly simply
>> > bypasses some audit-related stuff.
>> 
>> Hmmm, maybe I wasn't explicit enough about it. We are definitely
>> seeing a memory leak without the patch.
>> 
>> When auditing is enabled, putname() calls audit_putname *instead* (not
>> in addition) to __putname(). Then, if a syscall is in progress,
>> audit_putname does not release the name - instead, it expects the name
>> to get released when the syscall completes, but that will happen only
>> if audit_getname() was called previously, i.e. if the name was
>> allocated with getname() rather than the naked __getname(). So,
>> __getname() followed by putname() ends up leaking memory.
>> 
>
> OK.  Please resend with a new changelog?

This seems like a reasonable change to me.  I guess I misunderstood
the __getname/putname interaction.  I expect I was focusing on the
fact you can only call getname if you have your data in userspace.

> The bug surprises me - it seems that it makes it trivial for userspace
> to cause leaking of mad amounts of kernel memory, which would cause the
> bug to be found and fixed quickly.
>
> Is it a recent regression, or does the bug trigger only in weird
> circumstances, or what?

Calling sysctl(2) is very rare.  I don't know if it actually happens
anywhere with a modern userspace except in regression tests.
Effectively we retain sysctl(2) because it doesn't take too much to
maintain.

Michel what caused you to discover this bug?  If you are using
sysctl(2) in production code I am a bit worried.

Eric
--
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/