From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752670Ab2KRTev (ORCPT ); Sun, 18 Nov 2012 14:34:51 -0500 Received: from mail-oa0-f46.google.com ([209.85.219.46]:59217 "EHLO mail-oa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752459Ab2KRTet (ORCPT ); Sun, 18 Nov 2012 14:34:49 -0500 MIME-Version: 1.0 In-Reply-To: References: <20121024232032.GA31129@www.outflux.net> <20121025041620.GH2616@ZenIV.linux.org.uk> <20121025120952.GI2616@ZenIV.linux.org.uk> <20121025123843.GJ2616@ZenIV.linux.org.uk> <20121026183601.GR2616@ZenIV.linux.org.uk> Date: Sun, 18 Nov 2012 11:34:48 -0800 X-Google-Sender-Auth: biPUU25AaxbgQp6o7GSnI4B7v94 Message-ID: Subject: Re: [PATCH] exec: do not leave bprm->interp on stack From: Kees Cook To: P J P Cc: Al Viro , linux-kernel@vger.kernel.org, Andrew Morton , Josh Triplett , Serge Hallyn , linux-fsdevel@vger.kernel.org, halfdog Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Nov 18, 2012 at 11:04 AM, P J P 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