linux-hardening - RE: [PATCH v2] usercopy: Check valid lifetime via stack depth

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <85d42900efaa4fdb8c20de2147d938c7@AcuMS.aculab.com>
Date:   Thu, 24 Feb 2022 08:58:20 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Kees Cook' <keescook@...omium.org>,
        Matthew Wilcox <willy@...radead.org>
CC:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Muhammad Usama Anjum <usama.anjum@...labora.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "linux-sh@...r.kernel.org" <linux-sh@...r.kernel.org>,
        "linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
Subject: RE: [PATCH v2] usercopy: Check valid lifetime via stack depth

From: Kees Cook
> Sent: 24 February 2022 06:04
> 
> Under CONFIG_HARDENED_USERCOPY=y, when exact stack frame boundary checking
> is not available (i.e. everything except x86 with FRAME_POINTER), check
> a stack object as being at least "current depth valid", in the sense
> that any object within the stack region but not between start-of-stack
> and current_stack_pointer should be considered unavailable (i.e. its
> lifetime is from a call no longer present on the stack).
> 
...
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index d0d268135d96..5d28725af95f 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -22,6 +22,30 @@
>  #include <asm/sections.h>
>  #include "slab.h"
> 
> +/*
> + * Only called if obj is within stack/stackend bounds. Determine if within
> + * current stack depth.
> + */
> +static inline int check_stack_object_depth(const void *obj,
> +					   unsigned long len)
> +{
> +#ifdef CONFIG_ARCH_HAS_CURRENT_STACK_POINTER
> +#ifndef CONFIG_STACK_GROWSUP

Pointless negation

> +	const void * const high = stackend;
> +	const void * const low = (void *)current_stack_pointer;
> +#else
> +	const void * const high = (void *)current_stack_pointer;
> +	const void * const low = stack;
> +#endif
> +
> +	/* Reject: object not within current stack depth. */
> +	if (obj < low || high < obj + len)
> +		return BAD_STACK;
> +
> +#endif
> +	return GOOD_STACK;
> +}

If the comment at the top of the function is correct then
only a single test for the correct end of the buffer against
the current stack pointer is needed.
Something like:
#ifdef CONFIG_STACK_GROWSUP
	if ((void *)current_stack_pointer < obj + len)
		return BAD_STACK;
#else
	if (obj < (void *)current_stack_pointer)
		return BAD_STACK;
#endif
	return GOOD_STACK;

Although it may depend on exactly where the stack pointer
points to - especially for GROWSUP.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)