[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 7 Jul 2016 09:42:17 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Kees Cook <keescook@...omium.org>
cc: linux-kernel@...r.kernel.org, Rik van Riel <riel@...hat.com>,
Casey Schaufler <casey@...aufler-ca.com>, PaX Team <pageexec@...email.hu>,
Brad Spengler <spender@...ecurity.net>,
Russell King <linux@...linux.org.uk>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will.deacon@....com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Michael Ellerman <mpe@...erman.id.au>, Tony Luck <tony.luck@...el.com>,
Fenghua Yu <fenghua.yu@...el.com>, "David S. Miller" <davem@...emloft.net>,
x86@...nel.org, Christoph Lameter <cl@...ux.com>,
Pekka Enberg <penberg@...nel.org>, David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Andy Lutomirski <luto@...nel.org>, Borislav Petkov <bp@...e.de>,
Mathias Krause <minipli@...glemail.com>, Jan Kara <jack@...e.cz>,
Vitaly Wool <vitalywool@...il.com>, Andrea Arcangeli <aarcange@...hat.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Laura Abbott <labbott@...oraproject.org>,
linux-arm-kernel@...ts.infradead.org, linux-ia64@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, sparclinux@...r.kernel.org,
linux-arch@...r.kernel.org, linux-mm@...ck.org,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH 1/9] mm: Hardened usercopy
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
