From: halfdog <me@halfdog.net>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Search for patch for kernel stack data disclosure in binfmt_script during execve
Date: Fri, 24 Aug 2012 10:10:10 +0000 [thread overview]
Message-ID: <50375302.1050603@halfdog.net> (raw)
In-Reply-To: <20120823085625.GA4683@otc-wbsnb-06>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Kirill A. Shutemov wrote:
> On Wed, Aug 22, 2012 at 09:49:35PM +0000, halfdog wrote:
>> Got a hint via IRC, that I should not send patch idea for review
>> to "generic" list, but to maintainers and last (or relevant)
>> comitters of code.
>>
>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=commitdiff;h=bf2a9a39639b8b51377905397a5005f444e9a892
>>
>>
...
>> halfdog wrote:
>>> halfdog wrote:
>>>> I'm searching for a patch for linux kernel stack disclosure
>>>> in binfmt_script with crafted interpreter names when
>>>> CONFIG_MODULES is active (see [1]).
>>>
>>> Please disregard my previous proposal [2], since it did not
>>> address the problem directly (referencing local stack frame
>>> data from bprm structure) but worked around it. I suspect,
>>> that this could increase probability to reintroduce similar
>>> bugs.
>>>
>>> Opinions on (untested sketch for) second solution: Could
>>> someone look on the source code comments and changes in patch
>>> to judge, if this is going in the right direction?
>>>
>>> Explanation of patch: Since load_script will start to
>>> irreversibly change bprm structures at some point (using stack
>>> local data was one of those changes), try to delay this point.
>>> Run checks if load_script could be the right handler, if not
>>> give other binfmt handlers the chance to do so.
>>>
>>> If binfmt_script is the right one, try to load the interpreter
>>> (causing bprm modification), if failing make sure that no
>>> other binfmt handler has the chance to continue on the now
>>> modified bprm data.
>>>
>>> CAVEAT: This assumes, that if binfmt_script could handle the
>>> load, that it would be the one and only binfmt with that
>>> ability, so no other one, e.g. binfmt_misc should have the
>>> chance to do so. If this assumption is wrong, leaving
>>> binfmt_script would have to rollback all bprm changes (e.g.
>>> restore old credentials).
>>>
>>> [1]
>>> http://www.halfdog.net/Security/2012/LinuxKernelBinfmtScriptStackDataDisclosure/
>>>
>>>
[2] http://lkml.org/lkml/2012/8/18/75
>
> What about (untested):
>
> diff --git a/fs/exec.c b/fs/exec.c index 574cf4d..ef13850 100644
> --- a/fs/exec.c +++ b/fs/exec.c @@ -1438,7 +1438,8 @@ int
> search_binary_handler(struct linux_binprm *bprm,struct pt_regs
> *regs) } read_unlock(&binfmt_lock); #ifdef CONFIG_MODULES - if
> (retval != -ENOEXEC || bprm->mm == NULL) { + if (retval !=
> -ENOEXEC || bprm->mm == NULL || + bprm->recursion_depth >
> BINPRM_MAX_RECURSION) { break; } else { #define printable(c)
> (((c)=='\t') || ((c)=='\n') || (0x20<=(c) && (c)<=0x7e))
- - From my understanding, this patch should not fix the problem, since
recursion depth is reset back to old value after call of binfmt handler.
This is done, so that fs/exec does not have to trust all binfmts to
reset the depth by themselfes when leaving.
http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=fs/exec.c;h=da27b91ff1e8cbe87d0fe42aa5d39513e6a9deeb;hb=HEAD
1408 read_unlock(&binfmt_lock);
1409 retval = fn(bprm, regs);
1410 /*
1411 * Restore the depth counter to its
starting value
1412 * in this call, so we don't have to
rely on every
1413 * load_binary function to restore it on
return.
1414 */
1415 bprm->recursion_depth = depth;
I guess, the problem is, that recursion_depth usually not only limits
the depth, but also the maximal number of binfmt_xxx calls. That's why,
the use of local stack-frame data in bprm does not matter, there is no
going up the stack AND using bprm->interpreter, the last error is
terminates the search.
In the POC, search is not terminated because of ENOEXEC when max depth
reached and due to special filename, mod-loader triggers also (about 30
times? I do not known, if that could be a problem also, interfering with
other module loads. Usually non-root users cannot trigger rapid module
loads easily).
- --
http://www.halfdog.net/
PGP: 156A AE98 B91F 0114 FE88 2BD8 C459 9386 feed a bee
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iEYEARECAAYFAlA3U3QACgkQxFmThv7tq+7hTgCZAcQFn70FUWnAhvoMYhm8EcFT
8vQAn06VtbeY5P0cPGd9fcxL6AaEF8oS
=An9g
-----END PGP SIGNATURE-----
next prev parent reply other threads:[~2012-08-24 10:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-18 14:00 Search for patch for kernel stack disclosure in binfmt_script during execve halfdog
2012-08-19 8:39 ` Search for patch for kernel stack data " halfdog
2012-08-22 21:49 ` halfdog
2012-08-23 8:56 ` Kirill A. Shutemov
2012-08-24 10:10 ` halfdog [this message]
2012-09-20 16:05 ` [PATCH] Fix " halfdog
2012-09-21 19:15 ` Randy Dunlap
2012-09-23 4:54 ` [PATCH v2] " halfdog
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50375302.1050603@halfdog.net \
--to=me@halfdog.net \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.