linux-kernel - Re: [PATCH] exec: do not leave bprm->interp on stack

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: <CAGXu5jLwvcpyMp8RcF8aFySKkkj+17ovNOvC_r88+D0fQW7CsQ@mail.gmail.com>
Date:	Wed, 24 Oct 2012 23:21:21 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Josh Triplett <josh@...htriplett.org>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	linux-fsdevel@...r.kernel.org, halfdog <me@...fdog.net>
Subject: Re: [PATCH] exec: do not leave bprm->interp on stack

On Wed, Oct 24, 2012 at 9:16 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Wed, Oct 24, 2012 at 04:20:32PM -0700, Kees Cook wrote:
>> If a series of scripts are executed, each triggering module loading via
>> unprintable bytes in the script header, kernel stack contents can leak
>> into the command line.
>>
>> Normally execution of binfmt_script and binfmt_misc happens
>> recursively. However, when modules are enabled, and unprintable bytes
>> exist in the bprm->buf, execution will restart after attempting to load
>> matching binfmt modules. Unfortunately, the logic in binfmt_script and
>> binfmt_misc does not expect to get restarted. They leave bprm->interp
>> pointing to their local stack. This means on restart bprm->interp is
>> left pointing into unused stack memory which can then be copied into
>> the userspace argv areas.
>>
>> This changes the logic to require allocation for any changes to the
>> bprm->interp. To avoid adding a new kmalloc to every exec, the default
>> value is left as-is. Only when passing through binfmt_script or
>> binfmt_misc does an allocation take place.
>
> I really don't like that.  It papers over the problem, but doesn't really
> solve the underlying stupidity.  We have no good reason to retry a binfmt
> we'd already attempted on this level of recursion.  And your patch doesn't
> deal with that at all.

What should the code here _actually_ be doing? The _script and _misc
handlers expect to rewrite the bprm contents and recurse, but the
module loader want to try again. It's not clear to me what the binfmt
module handler is even there for; I don't see any binfmt-XXXX aliases
in the tree. If nothing uses it, should we just rip it out? That would
solve it too.

-Kees

-- 
Kees Cook
Chrome OS Security
--
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/