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 > > CC to generic just for the records > > 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). > > > > hd > > > > [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))
--
Kirill A. Shutemov
signature.asc
Description: Digital signature
