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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJ4ZuviFMQcFocLWfcMreZSt4kCHFNm-PNW+yuy+v+FiQ@mail.gmail.com>
Date:	Sun, 18 Nov 2012 11:34:48 -0800
From:	Kees Cook <keescook@...omium.org>
To:	P J P <ppandit@...hat.com>
Cc:	Al Viro <viro@...iv.linux.org.uk>, 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 Sun, Nov 18, 2012 at 11:04 AM, P J P <ppandit@...hat.com> wrote:
> +-- On Fri, 16 Nov 2012, Kees Cook wrote --+
> | Hrm? It should be showing only the live heap-allocated interp -- are
> | you seeing uninitialized contents?
>
>  I don't see uninitialised content; I see interpreter names from previous
> iterations. Which was the case earlier as well. The - interp - array is
> initialised with the interpreter name, before being assigned to bprm->interp.
>
> These - interp - bytes are *leaked* because after 4 recursions, when
> load_script returns -ENOEXEC, - bprm->interp - becomes invalid for it starts
> pointing to an invalid stack memory location.
>
> Crux of the problem is in the fact that the recursion limit -
> BINPRM_MAX_RECURSION(4) - exceeds after ones been rightly adhered to.
>
>         (bprm->recursion_depth > BINPRM_MAX_RECURSION))
>                 return -ENOEXEC;
>
> This check fails due to specific condition, which still exists.
>
> Dynamically allocating memory fixes the leak by making the memory area live
> and valid.

Right. There are two problems. This fixes the first, which is the
memory content leak.

> It does not fix the problem which caused the leak in the first place by
> exceeding the BINPRM_MAX_RECURSION, not by 1 or 2 but possible 2^6
> recursions. Isn't that performance hit?

This is the second problem. I view this as less critical because it's
only 64 instead of 4, but it certainly should be solved as well.

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