All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Morse <james.morse@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: ard.biesheuvel@linaro.org, kernel-hardening@lists.openwall.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, akashi.takahiro@linaro.org,
	catalin.marinas@arm.com, dave.martin@arm.com,
	labbott@fedoraproject.org, will.deacon@arm.com,
	keescook@chromium.org
Subject: Re: [RFC PATCH 2/6] arm64: avoid open-coding THREAD_SIZE{,_ORDER}
Date: Thu, 13 Jul 2017 11:18:35 +0100	[thread overview]
Message-ID: <596748FB.20902@arm.com> (raw)
In-Reply-To: <1499898783-25732-3-git-send-email-mark.rutland@arm.com>

Hi Mark,

On 12/07/17 23:32, Mark Rutland wrote:
> Currently we define THREAD_SIZE_ORDER dependent on which arm64-specific
> page size kconfig symbol was selected. This is unfortunate, as it hides
> the relationship between THREAD_SIZE_ORDER and THREAD_SIZE, and makes it
> painful more painful than necessary to modify the thread size as we will
> need to do for some debug configurations.
> 
> This patch follows arch/metag's approach of consistently defining
> THREAD_SIZE in terms of THREAD_SIZE_ORDER. This avoids having ifdefs for
> particular page size configurations, and allows us to change a single
> definition to change the thread size.

I think this has unintended side effects for 64K page systems.  (or at least not
yet intended)

Today:
> #ifdef CONFIG_ARM64_4K_PAGES
> #define THREAD_SIZE_ORDER	2
> #elif defined(CONFIG_ARM64_16K_PAGES)
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is unset on 64K, and THREAD_SIZE is always:
> #define THREAD_SIZE		16384

/kernel/fork.c matches this with its:
> # if THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK)
[...]
> #else
[...]
> void thread_stack_cache_init(void)
> {
>	thread_stack_cache = kmem_cache_create("thread_stack", THREAD_SIZE,
> 					      THREAD_SIZE, 0, NULL);
> 	BUG_ON(thread_stack_cache == NULL);
> }
> #endif

To create a kmemcache to share 64K pages as 16K stacks.


After this patch:
> #define THREAD_SHIFT		14
>
> #if THREAD_SHIFT >= PAGE_SHIFT
> #define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> #else
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is 0, and:
> #define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)

gives us a 64K THREAD_SIZE.



Thanks,

James





> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 141f13e9..6d0c59a 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -23,13 +23,17 @@
>  
>  #include <linux/compiler.h>
>  
> -#ifdef CONFIG_ARM64_4K_PAGES
> -#define THREAD_SIZE_ORDER	2
> -#elif defined(CONFIG_ARM64_16K_PAGES)
> +#include <asm/page.h>
> +
> +#define THREAD_SHIFT		14
> +
> +#if THREAD_SHIFT >= PAGE_SHIFT
> +#define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> +#else
>  #define THREAD_SIZE_ORDER	0
>  #endif
>  
> -#define THREAD_SIZE		16384
> +#define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP		(THREAD_SIZE - 16)
>  
>  #ifndef __ASSEMBLY__
> 

WARNING: multiple messages have this Message-ID (diff)
From: james.morse@arm.com (James Morse)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 2/6] arm64: avoid open-coding THREAD_SIZE{,_ORDER}
Date: Thu, 13 Jul 2017 11:18:35 +0100	[thread overview]
Message-ID: <596748FB.20902@arm.com> (raw)
In-Reply-To: <1499898783-25732-3-git-send-email-mark.rutland@arm.com>

Hi Mark,

On 12/07/17 23:32, Mark Rutland wrote:
> Currently we define THREAD_SIZE_ORDER dependent on which arm64-specific
> page size kconfig symbol was selected. This is unfortunate, as it hides
> the relationship between THREAD_SIZE_ORDER and THREAD_SIZE, and makes it
> painful more painful than necessary to modify the thread size as we will
> need to do for some debug configurations.
> 
> This patch follows arch/metag's approach of consistently defining
> THREAD_SIZE in terms of THREAD_SIZE_ORDER. This avoids having ifdefs for
> particular page size configurations, and allows us to change a single
> definition to change the thread size.

I think this has unintended side effects for 64K page systems.  (or at least not
yet intended)

Today:
> #ifdef CONFIG_ARM64_4K_PAGES
> #define THREAD_SIZE_ORDER	2
> #elif defined(CONFIG_ARM64_16K_PAGES)
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is unset on 64K, and THREAD_SIZE is always:
> #define THREAD_SIZE		16384

/kernel/fork.c matches this with its:
> # if THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK)
[...]
> #else
[...]
> void thread_stack_cache_init(void)
> {
>	thread_stack_cache = kmem_cache_create("thread_stack", THREAD_SIZE,
> 					      THREAD_SIZE, 0, NULL);
> 	BUG_ON(thread_stack_cache == NULL);
> }
> #endif

To create a kmemcache to share 64K pages as 16K stacks.


After this patch:
> #define THREAD_SHIFT		14
>
> #if THREAD_SHIFT >= PAGE_SHIFT
> #define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> #else
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is 0, and:
> #define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)

gives us a 64K THREAD_SIZE.



Thanks,

James





> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 141f13e9..6d0c59a 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -23,13 +23,17 @@
>  
>  #include <linux/compiler.h>
>  
> -#ifdef CONFIG_ARM64_4K_PAGES
> -#define THREAD_SIZE_ORDER	2
> -#elif defined(CONFIG_ARM64_16K_PAGES)
> +#include <asm/page.h>
> +
> +#define THREAD_SHIFT		14
> +
> +#if THREAD_SHIFT >= PAGE_SHIFT
> +#define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> +#else
>  #define THREAD_SIZE_ORDER	0
>  #endif
>  
> -#define THREAD_SIZE		16384
> +#define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP		(THREAD_SIZE - 16)
>  
>  #ifndef __ASSEMBLY__
> 

WARNING: multiple messages have this Message-ID (diff)
From: James Morse <james.morse@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: ard.biesheuvel@linaro.org, kernel-hardening@lists.openwall.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, akashi.takahiro@linaro.org,
	catalin.marinas@arm.com, dave.martin@arm.com,
	labbott@fedoraproject.org, will.deacon@arm.com,
	keescook@chromium.org
Subject: [kernel-hardening] Re: [RFC PATCH 2/6] arm64: avoid open-coding THREAD_SIZE{,_ORDER}
Date: Thu, 13 Jul 2017 11:18:35 +0100	[thread overview]
Message-ID: <596748FB.20902@arm.com> (raw)
In-Reply-To: <1499898783-25732-3-git-send-email-mark.rutland@arm.com>

Hi Mark,

On 12/07/17 23:32, Mark Rutland wrote:
> Currently we define THREAD_SIZE_ORDER dependent on which arm64-specific
> page size kconfig symbol was selected. This is unfortunate, as it hides
> the relationship between THREAD_SIZE_ORDER and THREAD_SIZE, and makes it
> painful more painful than necessary to modify the thread size as we will
> need to do for some debug configurations.
> 
> This patch follows arch/metag's approach of consistently defining
> THREAD_SIZE in terms of THREAD_SIZE_ORDER. This avoids having ifdefs for
> particular page size configurations, and allows us to change a single
> definition to change the thread size.

I think this has unintended side effects for 64K page systems.  (or at least not
yet intended)

Today:
> #ifdef CONFIG_ARM64_4K_PAGES
> #define THREAD_SIZE_ORDER	2
> #elif defined(CONFIG_ARM64_16K_PAGES)
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is unset on 64K, and THREAD_SIZE is always:
> #define THREAD_SIZE		16384

/kernel/fork.c matches this with its:
> # if THREAD_SIZE >= PAGE_SIZE || defined(CONFIG_VMAP_STACK)
[...]
> #else
[...]
> void thread_stack_cache_init(void)
> {
>	thread_stack_cache = kmem_cache_create("thread_stack", THREAD_SIZE,
> 					      THREAD_SIZE, 0, NULL);
> 	BUG_ON(thread_stack_cache == NULL);
> }
> #endif

To create a kmemcache to share 64K pages as 16K stacks.


After this patch:
> #define THREAD_SHIFT		14
>
> #if THREAD_SHIFT >= PAGE_SHIFT
> #define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> #else
> #define THREAD_SIZE_ORDER	0
> #endif

Means THREAD_SIZE_ORDER is 0, and:
> #define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)

gives us a 64K THREAD_SIZE.



Thanks,

James





> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 141f13e9..6d0c59a 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -23,13 +23,17 @@
>  
>  #include <linux/compiler.h>
>  
> -#ifdef CONFIG_ARM64_4K_PAGES
> -#define THREAD_SIZE_ORDER	2
> -#elif defined(CONFIG_ARM64_16K_PAGES)
> +#include <asm/page.h>
> +
> +#define THREAD_SHIFT		14
> +
> +#if THREAD_SHIFT >= PAGE_SHIFT
> +#define THREAD_SIZE_ORDER	(THREAD_SHIFT - PAGE_SHIFT)
> +#else
>  #define THREAD_SIZE_ORDER	0
>  #endif
>  
> -#define THREAD_SIZE		16384
> +#define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP		(THREAD_SIZE - 16)
>  
>  #ifndef __ASSEMBLY__
> 

  reply	other threads:[~2017-07-13 10:19 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-12 22:32 [RFC PATCH 0/6] arm64: alternative VMAP_STACK implementation Mark Rutland
2017-07-12 22:32 ` [kernel-hardening] " Mark Rutland
2017-07-12 22:32 ` Mark Rutland
2017-07-12 22:32 ` [RFC PATCH 1/6] arm64: use tpidr_el1 for current, free sp_el0 Mark Rutland
2017-07-12 22:32   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:32   ` Mark Rutland
2017-07-14  1:30   ` Will Deacon
2017-07-14  1:30     ` [kernel-hardening] " Will Deacon
2017-07-14  1:30     ` Will Deacon
2017-07-12 22:32 ` [RFC PATCH 2/6] arm64: avoid open-coding THREAD_SIZE{,_ORDER} Mark Rutland
2017-07-12 22:32   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:32   ` Mark Rutland
2017-07-13 10:18   ` James Morse [this message]
2017-07-13 10:18     ` [kernel-hardening] " James Morse
2017-07-13 10:18     ` James Morse
2017-07-13 11:26     ` Mark Rutland
2017-07-13 11:26       ` [kernel-hardening] " Mark Rutland
2017-07-13 11:26       ` Mark Rutland
2017-07-12 22:33 ` [RFC PATCH 3/6] arm64: pad stacks to PAGE_SIZE for VMAP_STACK Mark Rutland
2017-07-12 22:33   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:33   ` Mark Rutland
2017-07-12 22:33 ` [RFC PATCH 4/6] arm64: pass stack base to secondary_start_kernel Mark Rutland
2017-07-12 22:33   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:33   ` Mark Rutland
2017-07-12 22:33 ` [RFC PATCH 5/6] arm64: keep track of current stack Mark Rutland
2017-07-12 22:33   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:33   ` Mark Rutland
2017-07-12 22:33 ` [RFC PATCH 6/6] arm64: add VMAP_STACK and detect out-of-bounds SP Mark Rutland
2017-07-12 22:33   ` [kernel-hardening] " Mark Rutland
2017-07-12 22:33   ` Mark Rutland
2017-07-13  6:58   ` Ard Biesheuvel
2017-07-13  6:58     ` [kernel-hardening] " Ard Biesheuvel
2017-07-13  6:58     ` Ard Biesheuvel
2017-07-13 10:49     ` Mark Rutland
2017-07-13 10:49       ` [kernel-hardening] " Mark Rutland
2017-07-13 10:49       ` Mark Rutland
2017-07-13 11:49       ` Ard Biesheuvel
2017-07-13 11:49         ` [kernel-hardening] " Ard Biesheuvel
2017-07-13 11:49         ` Ard Biesheuvel
2017-07-13 16:10         ` Mark Rutland
2017-07-13 16:10           ` [kernel-hardening] " Mark Rutland
2017-07-13 16:10           ` Mark Rutland
2017-07-13 17:55           ` [kernel-hardening] " Mark Rutland
2017-07-13 17:55             ` Mark Rutland
2017-07-13 17:55             ` Mark Rutland
2017-07-13 18:28             ` Ard Biesheuvel
2017-07-13 18:28               ` Ard Biesheuvel
2017-07-13 18:28               ` Ard Biesheuvel
2017-07-14 10:32               ` Mark Rutland
2017-07-14 10:32                 ` Mark Rutland
2017-07-14 10:32                 ` Mark Rutland
2017-07-14 10:48                 ` Ard Biesheuvel
2017-07-14 10:48                   ` Ard Biesheuvel
2017-07-14 10:48                   ` Ard Biesheuvel
2017-07-14 12:27                   ` Ard Biesheuvel
2017-07-14 12:27                     ` Ard Biesheuvel
2017-07-14 12:27                     ` Ard Biesheuvel
2017-07-14 14:06                     ` Mark Rutland
2017-07-14 14:06                       ` Mark Rutland
2017-07-14 14:06                       ` Mark Rutland
2017-07-14 14:14                       ` Ard Biesheuvel
2017-07-14 14:14                         ` Ard Biesheuvel
2017-07-14 14:14                         ` Ard Biesheuvel
2017-07-14 14:39                       ` Robin Murphy
2017-07-14 14:39                         ` Robin Murphy
2017-07-14 14:39                         ` Robin Murphy
2017-07-14 15:03                         ` Robin Murphy
2017-07-14 15:03                           ` Robin Murphy
2017-07-14 15:03                           ` Robin Murphy
2017-07-14 15:15                           ` Ard Biesheuvel
2017-07-14 15:15                             ` Ard Biesheuvel
2017-07-14 15:15                             ` Ard Biesheuvel
2017-07-14 15:25                           ` Mark Rutland
2017-07-14 15:25                             ` Mark Rutland
2017-07-14 15:25                             ` Mark Rutland
2017-07-14 21:27                       ` Mark Rutland
2017-07-14 21:27                         ` Mark Rutland
2017-07-14 21:27                         ` Mark Rutland
2017-07-16  0:03                         ` Ard Biesheuvel
2017-07-16  0:03                           ` Ard Biesheuvel
2017-07-16  0:03                           ` Ard Biesheuvel
2017-07-18 21:53                           ` Laura Abbott
2017-07-18 21:53                             ` Laura Abbott
2017-07-18 21:53                             ` Laura Abbott
2017-07-19  8:08                             ` Ard Biesheuvel
2017-07-19  8:08                               ` Ard Biesheuvel
2017-07-19  8:08                               ` Ard Biesheuvel
2017-07-19 23:32                               ` Laura Abbott
2017-07-19 23:32                                 ` Laura Abbott
2017-07-20  5:35                                 ` Ard Biesheuvel
2017-07-20  5:35                                   ` Ard Biesheuvel
2017-07-20  5:35                                   ` Ard Biesheuvel
2017-07-20  8:36                                   ` James Morse
2017-07-20  8:36                                     ` James Morse
2017-07-20  8:36                                     ` James Morse
2017-07-20  8:56                                     ` Ard Biesheuvel
2017-07-20  8:56                                       ` Ard Biesheuvel
2017-07-20  8:56                                       ` Ard Biesheuvel
2017-07-20 17:30                                       ` Ard Biesheuvel
2017-07-20 17:30                                         ` Ard Biesheuvel
2017-07-20 17:30                                         ` Ard Biesheuvel
2017-07-20 19:10                                         ` Laura Abbott
2017-07-20 19:10                                           ` Laura Abbott
2017-07-20 19:10                                           ` Laura Abbott
2017-07-14 12:52                   ` Mark Rutland
2017-07-14 12:52                     ` Mark Rutland
2017-07-14 12:52                     ` Mark Rutland
2017-07-14 12:55                     ` Ard Biesheuvel
2017-07-14 12:55                       ` Ard Biesheuvel
2017-07-14 12:55                       ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=596748FB.20902@arm.com \
    --to=james.morse@arm.com \
    --cc=akashi.takahiro@linaro.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=dave.martin@arm.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=labbott@fedoraproject.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.