From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-f72.google.com (mail-lf0-f72.google.com [209.85.215.72]) by kanga.kvack.org (Postfix) with ESMTP id A954A6B0253 for ; Thu, 7 Jul 2016 03:45:42 -0400 (EDT) Received: by mail-lf0-f72.google.com with SMTP id g18so5959646lfg.2 for ; Thu, 07 Jul 2016 00:45:42 -0700 (PDT) Received: from Galois.linutronix.de (linutronix.de. [2001:470:1f0b:db:abcd:42:0:1]) by mx.google.com with ESMTPS id v139si2020186wmv.78.2016.07.07.00.45.41 for (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 07 Jul 2016 00:45:41 -0700 (PDT) Date: Thu, 7 Jul 2016 09:42:17 +0200 (CEST) From: Thomas Gleixner Subject: Re: [PATCH 1/9] mm: Hardened usercopy In-Reply-To: <1467843928-29351-2-git-send-email-keescook@chromium.org> Message-ID: References: <1467843928-29351-1-git-send-email-keescook@chromium.org> <1467843928-29351-2-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: Kees Cook Cc: linux-kernel@vger.kernel.org, Rik van Riel , Casey Schaufler , PaX Team , Brad Spengler , Russell King , Catalin Marinas , Will Deacon , Ard Biesheuvel , Benjamin Herrenschmidt , Michael Ellerman , Tony Luck , Fenghua Yu , "David S. Miller" , x86@kernel.org, Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Andy Lutomirski , Borislav Petkov , Mathias Krause , Jan Kara , Vitaly Wool , Andrea Arcangeli , Dmitry Vyukov , Laura Abbott , linux-arm-kernel@lists.infradead.org, linux-ia64@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, sparclinux@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, kernel-hardening@lists.openwall.com On Wed, 6 Jul 2016, Kees Cook wrote: > + > +#if defined(CONFIG_FRAME_POINTER) && defined(CONFIG_X86) > + const void *frame = NULL; > + const void *oldframe; > +#endif That's ugly > + > + /* Object is not on the stack at all. */ > + if (obj + len <= stack || stackend <= obj) > + return 0; > + > + /* > + * Reject: object partially overlaps the stack (passing the > + * the check above means at least one end is within the stack, > + * so if this check fails, the other end is outside the stack). > + */ > + if (obj < stack || stackend < obj + len) > + return -1; > + > +#if defined(CONFIG_FRAME_POINTER) && defined(CONFIG_X86) > + oldframe = __builtin_frame_address(1); > + if (oldframe) > + frame = __builtin_frame_address(2); > + /* > + * low ----------------------------------------------> high > + * [saved bp][saved ip][args][local vars][saved bp][saved ip] > + * ^----------------^ > + * allow copies only within here > + */ > + while (stack <= frame && frame < stackend) { > + /* > + * If obj + len extends past the last frame, this > + * check won't pass and the next frame will be 0, > + * causing us to bail out and correctly report > + * the copy as invalid. > + */ > + if (obj + len <= frame) > + return obj >= oldframe + 2 * sizeof(void *) ? 2 : -1; > + oldframe = frame; > + frame = *(const void * const *)frame; > + } > + return -1; > +#else > + return 1; > +#endif I'd rather make that a weak function returning 1 which can be replaced by x86 for CONFIG_FRAME_POINTER=y. That also allows other architectures to implement their specific frame checks. Thanks, tglx -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org