public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
  2025-01-22 17:19 [PATCH v2 " Mel Gorman
@ 2025-01-22 17:19 ` Mel Gorman
  2025-01-23  1:01   ` Kees Cook
  0 siblings, 1 reply; 10+ messages in thread
From: Mel Gorman @ 2025-01-22 17:19 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

HARDENED_USERCOPY is checked within a function so even if disabled, the
function overhead still exists. Move the static check inline.

This is at best a micro-optimisation and any difference in performance
was within noise but it is relatively consistent with the init_on_*
implementations.

Suggested-by: Kees Cook <kees@kernel.org>
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 include/linux/thread_info.h |  8 ++++++++
 mm/usercopy.c               | 11 ++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cf2446c9c30d..832f6a97e64c 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -221,9 +221,17 @@ static inline int arch_within_stack_frames(const void * const stack,
 extern void __check_object_size(const void *ptr, unsigned long n,
 					bool to_user);
 
+DECLARE_STATIC_KEY_MAYBE(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+			   validate_usercopy_range);
+
 static __always_inline void check_object_size(const void *ptr, unsigned long n,
 					      bool to_user)
 {
+	if (static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+				&validate_usercopy_range)) {
+		return;
+	}
+
 	if (!__builtin_constant_p(n))
 		__check_object_size(ptr, n, to_user);
 }
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 4cf33305347a..2e86413ed244 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -201,7 +201,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
 	}
 }
 
-static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
+DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+			   validate_usercopy_range);
+EXPORT_SYMBOL(validate_usercopy_range);
 
 /*
  * Validates that the given object is:
@@ -212,9 +214,6 @@ static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
  */
 void __check_object_size(const void *ptr, unsigned long n, bool to_user)
 {
-	if (static_branch_unlikely(&bypass_usercopy_checks))
-		return;
-
 	/* Skip all tests if size is zero. */
 	if (!n)
 		return;
@@ -271,7 +270,9 @@ __setup("hardened_usercopy=", parse_hardened_usercopy);
 static int __init set_hardened_usercopy(void)
 {
 	if (enable_checks == false)
-		static_branch_enable(&bypass_usercopy_checks);
+		static_branch_enable(&validate_usercopy_range);
+	else
+		static_branch_disable(&validate_usercopy_range);
 	return 1;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
  2025-01-22 17:19 ` [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled Mel Gorman
@ 2025-01-23  1:01   ` Kees Cook
  2025-01-23 11:47     ` Mel Gorman
  0 siblings, 1 reply; 10+ messages in thread
From: Kees Cook @ 2025-01-23  1:01 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel

On Wed, Jan 22, 2025 at 05:19:24PM +0000, Mel Gorman wrote:
> HARDENED_USERCOPY is checked within a function so even if disabled, the
> function overhead still exists. Move the static check inline.
> 
> This is at best a micro-optimisation and any difference in performance
> was within noise but it is relatively consistent with the init_on_*
> implementations.
> 
> Suggested-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> ---
>  include/linux/thread_info.h |  8 ++++++++
>  mm/usercopy.c               | 11 ++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> index cf2446c9c30d..832f6a97e64c 100644
> --- a/include/linux/thread_info.h
> +++ b/include/linux/thread_info.h
> @@ -221,9 +221,17 @@ static inline int arch_within_stack_frames(const void * const stack,
>  extern void __check_object_size(const void *ptr, unsigned long n,
>  					bool to_user);
>  
> +DECLARE_STATIC_KEY_MAYBE(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> +			   validate_usercopy_range);

This should be DECLARE_STATIC_KEY_MAYBE_RO()

>  static __always_inline void check_object_size(const void *ptr, unsigned long n,
>  					      bool to_user)
>  {
> +	if (static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> +				&validate_usercopy_range)) {
> +		return;
> +	}
> +
>  	if (!__builtin_constant_p(n))
>  		__check_object_size(ptr, n, to_user);
>  }

This is accidentally correct ("if validate, skip" matches "if not
enabled, disable validation" below, but is very confusing. Also, yes,
this is good to be moved into the inline, but let's wrap it in the
compile-time __builtin_constant_p() check:

static __always_inline void check_object_size(const void *ptr, unsigned long n,
					      bool to_user)
{
	if (!__builtin_constant_p(n) &&
	    static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
				&validate_usercopy_range))
		__check_object_size(ptr, n, to_user);
}

> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index 4cf33305347a..2e86413ed244 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -201,7 +201,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
>  	}
>  }
>  
> -static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
> +DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> +			   validate_usercopy_range);
> +EXPORT_SYMBOL(validate_usercopy_range);
>  
>  /*
>   * Validates that the given object is:
> @@ -212,9 +214,6 @@ static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
>   */
>  void __check_object_size(const void *ptr, unsigned long n, bool to_user)
>  {
> -	if (static_branch_unlikely(&bypass_usercopy_checks))
> -		return;
> -
>  	/* Skip all tests if size is zero. */
>  	if (!n)
>  		return;
> @@ -271,7 +270,9 @@ __setup("hardened_usercopy=", parse_hardened_usercopy);
>  static int __init set_hardened_usercopy(void)
>  {
>  	if (enable_checks == false)
> -		static_branch_enable(&bypass_usercopy_checks);
> +		static_branch_enable(&validate_usercopy_range);
> +	else
> +		static_branch_disable(&validate_usercopy_range);

This should be:

	if (enable_checks)
		static_branch_enable(&validate_usercopy_range);
	else
		static_branch_disable(&validate_usercopy_range);

>  	return 1;
>  }
>  
> -- 
> 2.43.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
  2025-01-23  1:01   ` Kees Cook
@ 2025-01-23 11:47     ` Mel Gorman
  0 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 11:47 UTC (permalink / raw)
  To: Kees Cook; +Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel

On Wed, Jan 22, 2025 at 05:01:21PM -0800, Kees Cook wrote:
> On Wed, Jan 22, 2025 at 05:19:24PM +0000, Mel Gorman wrote:
> > HARDENED_USERCOPY is checked within a function so even if disabled, the
> > function overhead still exists. Move the static check inline.
> > 
> > This is at best a micro-optimisation and any difference in performance
> > was within noise but it is relatively consistent with the init_on_*
> > implementations.
> > 
> > Suggested-by: Kees Cook <kees@kernel.org>
> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
> > ---
> >  include/linux/thread_info.h |  8 ++++++++
> >  mm/usercopy.c               | 11 ++++++-----
> >  2 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
> > index cf2446c9c30d..832f6a97e64c 100644
> > --- a/include/linux/thread_info.h
> > +++ b/include/linux/thread_info.h
> > @@ -221,9 +221,17 @@ static inline int arch_within_stack_frames(const void * const stack,
> >  extern void __check_object_size(const void *ptr, unsigned long n,
> >  					bool to_user);
> >  
> > +DECLARE_STATIC_KEY_MAYBE(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> > +			   validate_usercopy_range);
> 
> This should be DECLARE_STATIC_KEY_MAYBE_RO()
> 

Doesn't exist. Are you mixing it up with the DEFINE_STATIC_KEY_MAYBE_RO?


> >  static __always_inline void check_object_size(const void *ptr, unsigned long n,
> >  					      bool to_user)
> >  {
> > +	if (static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> > +				&validate_usercopy_range)) {
> > +		return;
> > +	}
> > +
> >  	if (!__builtin_constant_p(n))
> >  		__check_object_size(ptr, n, to_user);
> >  }
> 
> This is accidentally correct ("if validate, skip" matches "if not
> enabled, disable validation" below, but is very confusing. Also, yes,
> this is good to be moved into the inline, but let's wrap it in the
> compile-time __builtin_constant_p() check:
> 
> static __always_inline void check_object_size(const void *ptr, unsigned long n,
> 					      bool to_user)
> {
> 	if (!__builtin_constant_p(n) &&
> 	    static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> 				&validate_usercopy_range))
> 		__check_object_size(ptr, n, to_user);
> }
> 

Ok, should be fine given that it's a compile-time check anyway.

> > diff --git a/mm/usercopy.c b/mm/usercopy.c
> > index 4cf33305347a..2e86413ed244 100644
> > --- a/mm/usercopy.c
> > +++ b/mm/usercopy.c
> > @@ -201,7 +201,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
> >  	}
> >  }
> >  
> > -static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
> > +DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
> > +			   validate_usercopy_range);
> > +EXPORT_SYMBOL(validate_usercopy_range);
> >  
> >  /*
> >   * Validates that the given object is:
> > @@ -212,9 +214,6 @@ static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
> >   */
> >  void __check_object_size(const void *ptr, unsigned long n, bool to_user)
> >  {
> > -	if (static_branch_unlikely(&bypass_usercopy_checks))
> > -		return;
> > -
> >  	/* Skip all tests if size is zero. */
> >  	if (!n)
> >  		return;
> > @@ -271,7 +270,9 @@ __setup("hardened_usercopy=", parse_hardened_usercopy);
> >  static int __init set_hardened_usercopy(void)
> >  {
> >  	if (enable_checks == false)
> > -		static_branch_enable(&bypass_usercopy_checks);
> > +		static_branch_enable(&validate_usercopy_range);
> > +	else
> > +		static_branch_disable(&validate_usercopy_range);
> 
> This should be:
> 
> 	if (enable_checks)
> 		static_branch_enable(&validate_usercopy_range);
> 	else
> 		static_branch_disable(&validate_usercopy_range);
> 

Bah, fixed.

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time
@ 2025-01-23 22:11 Mel Gorman
  2025-01-23 22:11 ` [PATCH 1/4] mm: security: Move hardened usercopy under 'Kernel hardening options' Mel Gorman
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 22:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

Changelog since v2
o Default on
o Logic correction and simplification

Changelog since v1
o Menu section rename
o Make static branch usage similar to init_on_alloc
o Change ordering of menu options

Some hardening options like HARDENED_USERCOPY can be set at boot time
and have negligible cost when disabled. The default for options like
init_on_alloc= can be set at compile time but hardened usercopy is
enabled by default if built in. This incurs overhead when a kernel
wishes to provide optional hardening but the user does not necessarily
care.

Hardening is desirable in some environments but ideally they would be opt-in
by kernel command line as hardening is typically a deliberate decision
whereas the performance overhead is not always obvious to all users.
Patches 1 and 2 move HARDENED_USERCOPY to the Kconfig.hardening and
default it to disabled. Patch 3 moves the static branch check to a fast
path similar to init_on_*. Patch 4 moves FORTIFY_SOURCE to hardening only
because the option is related to hardening and happened to be declared
near HARDENED_USERCOPY.

Building HARDENED_USERCOPY but disabled at runtime has neligible effect
within the noise. Enabling the option by default generally incurs 2-10%
of overhead depending on the workload with some extreme outliers depending
on the exact CPU. While the benchmarks are somewhat synthetic, the overhead
IO-intensive and network-intensive is easily detectable but the root cause
may not be obvious (e.g. 2-14% overhead for netperf TCP_STREAM running
over localhost with different ranges depending on the CPU).

 .../admin-guide/kernel-parameters.txt         |  4 ++-
 include/linux/thread_info.h                   |  8 ++++-
 mm/usercopy.c                                 | 16 +++++----
 security/Kconfig                              | 21 ------------
 security/Kconfig.hardening                    | 33 +++++++++++++++++++
 5 files changed, 52 insertions(+), 30 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] mm: security: Move hardened usercopy under 'Kernel hardening options'
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
@ 2025-01-23 22:11 ` Mel Gorman
  2025-01-23 22:11 ` [PATCH 2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 22:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

There is a submenu for 'Kernel hardening options' under "Security".
Move HARDENED_USERCOPY under the hardening options as it is clearly
related.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Paul Moore <paul@paul-moore.com>
---
 security/Kconfig           | 12 ------------
 security/Kconfig.hardening | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index 28e685f53bd1..fe7346dc4bc3 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -159,18 +159,6 @@ config LSM_MMAP_MIN_ADDR
 	  this low address space will need the permission specific to the
 	  systems running LSM.
 
-config HARDENED_USERCOPY
-	bool "Harden memory copies between kernel and userspace"
-	imply STRICT_DEVMEM
-	help
-	  This option checks for obviously wrong memory regions when
-	  copying memory to/from the kernel (via copy_to_user() and
-	  copy_from_user() functions) by rejecting memory ranges that
-	  are larger than the specified heap object, span multiple
-	  separately allocated pages, are not on the process stack,
-	  or are part of the kernel text. This prevents entire classes
-	  of heap overflow exploits and similar kernel memory exposures.
-
 config FORTIFY_SOURCE
 	bool "Harden common str/mem functions against buffer overflows"
 	depends on ARCH_HAS_FORTIFY_SOURCE
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index c9d5ca3d8d08..9088d613d519 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -279,6 +279,22 @@ config ZERO_CALL_USED_REGS
 
 endmenu
 
+menu "Bounds checking"
+
+config HARDENED_USERCOPY
+	bool "Harden memory copies between kernel and userspace"
+	imply STRICT_DEVMEM
+	help
+	  This option checks for obviously wrong memory regions when
+	  copying memory to/from the kernel (via copy_to_user() and
+	  copy_from_user() functions) by rejecting memory ranges that
+	  are larger than the specified heap object, span multiple
+	  separately allocated pages, are not on the process stack,
+	  or are part of the kernel text. This prevents entire classes
+	  of heap overflow exploits and similar kernel memory exposures.
+
+endmenu
+
 menu "Hardening of kernel data structures"
 
 config LIST_HARDENED
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
  2025-01-23 22:11 ` [PATCH 1/4] mm: security: Move hardened usercopy under 'Kernel hardening options' Mel Gorman
@ 2025-01-23 22:11 ` Mel Gorman
  2025-01-23 22:11 ` [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled Mel Gorman
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 22:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

HARDENED_USERCOPY defaults to on if enabled at compile time. Allow
hardened_usercopy= default to be set at compile time similar to
init_on_alloc= and init_on_free=. The intent is that hardening
options that can be disabled at runtime can set their default at
build time.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 +++-
 mm/usercopy.c                                   | 3 ++-
 security/Kconfig.hardening                      | 8 ++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3872bc6ec49d..5d759b20540a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1773,7 +1773,9 @@
 			allocation boundaries as a proactive defense
 			against bounds-checking flaws in the kernel's
 			copy_to_user()/copy_from_user() interface.
-		on	Perform hardened usercopy checks (default).
+			The default is determined by
+			CONFIG_HARDENED_USERCOPY_DEFAULT_ON.
+		on	Perform hardened usercopy checks.
 		off	Disable hardened usercopy checks.
 
 	hardlockup_all_cpu_backtrace=
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 83c164aba6e0..4cf33305347a 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -255,7 +255,8 @@ void __check_object_size(const void *ptr, unsigned long n, bool to_user)
 }
 EXPORT_SYMBOL(__check_object_size);
 
-static bool enable_checks __initdata = true;
+static bool enable_checks __initdata =
+		IS_ENABLED(CONFIG_HARDENED_USERCOPY_DEFAULT_ON);
 
 static int __init parse_hardened_usercopy(char *str)
 {
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 9088d613d519..3a941cc45396 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -293,6 +293,14 @@ config HARDENED_USERCOPY
 	  or are part of the kernel text. This prevents entire classes
 	  of heap overflow exploits and similar kernel memory exposures.
 
+config HARDENED_USERCOPY_DEFAULT_ON
+	bool "Harden memory copies by default"
+	depends on HARDENED_USERCOPY
+	default HARDENED_USERCOPY
+	help
+	  This has the effect of setting "hardened_usercopy=on" on the kernel
+	  command line. This can be disabled with "hardened_usercopy=off".
+
 endmenu
 
 menu "Hardening of kernel data structures"
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
  2025-01-23 22:11 ` [PATCH 1/4] mm: security: Move hardened usercopy under 'Kernel hardening options' Mel Gorman
  2025-01-23 22:11 ` [PATCH 2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
@ 2025-01-23 22:11 ` Mel Gorman
  2025-01-23 22:11 ` [PATCH 4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options' Mel Gorman
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 22:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

HARDENED_USERCOPY is checked within a function so even if disabled, the
function overhead still exists. Move the static check inline.

This is at best a micro-optimisation and any difference in performance
was within noise but it is relatively consistent with the init_on_*
implementations.

Suggested-by: Kees Cook <kees@kernel.org>
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 include/linux/thread_info.h |  8 +++++++-
 mm/usercopy.c               | 13 +++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index cf2446c9c30d..09fc241a45f7 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -221,11 +221,17 @@ static inline int arch_within_stack_frames(const void * const stack,
 extern void __check_object_size(const void *ptr, unsigned long n,
 					bool to_user);
 
+DECLARE_STATIC_KEY_MAYBE(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+			   validate_usercopy_range);
+
 static __always_inline void check_object_size(const void *ptr, unsigned long n,
 					      bool to_user)
 {
-	if (!__builtin_constant_p(n))
+	if (!__builtin_constant_p(n) &&
+	    static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+				&validate_usercopy_range)) {
 		__check_object_size(ptr, n, to_user);
+	}
 }
 #else
 static inline void check_object_size(const void *ptr, unsigned long n,
diff --git a/mm/usercopy.c b/mm/usercopy.c
index 4cf33305347a..a1193bdabb16 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -201,7 +201,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
 	}
 }
 
-static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
+DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
+			   validate_usercopy_range);
+EXPORT_SYMBOL(validate_usercopy_range);
 
 /*
  * Validates that the given object is:
@@ -212,9 +214,6 @@ static DEFINE_STATIC_KEY_FALSE_RO(bypass_usercopy_checks);
  */
 void __check_object_size(const void *ptr, unsigned long n, bool to_user)
 {
-	if (static_branch_unlikely(&bypass_usercopy_checks))
-		return;
-
 	/* Skip all tests if size is zero. */
 	if (!n)
 		return;
@@ -270,8 +269,10 @@ __setup("hardened_usercopy=", parse_hardened_usercopy);
 
 static int __init set_hardened_usercopy(void)
 {
-	if (enable_checks == false)
-		static_branch_enable(&bypass_usercopy_checks);
+	if (enable_checks)
+		static_branch_enable(&validate_usercopy_range);
+	else
+		static_branch_disable(&validate_usercopy_range);
 	return 1;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options'
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
                   ` (2 preceding siblings ...)
  2025-01-23 22:11 ` [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled Mel Gorman
@ 2025-01-23 22:11 ` Mel Gorman
  2025-01-23 23:51 ` [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Kees Cook
  2025-02-11  0:45 ` Kees Cook
  5 siblings, 0 replies; 10+ messages in thread
From: Mel Gorman @ 2025-01-23 22:11 UTC (permalink / raw)
  To: Kees Cook
  Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel,
	Mel Gorman

FORTIFY_SOURCE is a hardening option both at build and runtime. Move
it under 'Kernel hardening options'.

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Acked-by: Paul Moore <paul@paul-moore.com>
---
 security/Kconfig           | 9 ---------
 security/Kconfig.hardening | 9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index fe7346dc4bc3..bca84f839fbe 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -159,15 +159,6 @@ config LSM_MMAP_MIN_ADDR
 	  this low address space will need the permission specific to the
 	  systems running LSM.
 
-config FORTIFY_SOURCE
-	bool "Harden common str/mem functions against buffer overflows"
-	depends on ARCH_HAS_FORTIFY_SOURCE
-	# https://github.com/llvm/llvm-project/issues/53645
-	depends on !CC_IS_CLANG || !X86_32
-	help
-	  Detect overflows of buffers in common string and memory functions
-	  where the compiler can determine and validate the buffer sizes.
-
 config STATIC_USERMODEHELPER
 	bool "Force all usermode helper calls through a single binary"
 	help
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 3a941cc45396..1ce9009c3cc6 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -281,6 +281,15 @@ endmenu
 
 menu "Bounds checking"
 
+config FORTIFY_SOURCE
+	bool "Harden common str/mem functions against buffer overflows"
+	depends on ARCH_HAS_FORTIFY_SOURCE
+	# https://github.com/llvm/llvm-project/issues/53645
+	depends on !CC_IS_CLANG || !X86_32
+	help
+	  Detect overflows of buffers in common string and memory functions
+	  where the compiler can determine and validate the buffer sizes.
+
 config HARDENED_USERCOPY
 	bool "Harden memory copies between kernel and userspace"
 	imply STRICT_DEVMEM
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
                   ` (3 preceding siblings ...)
  2025-01-23 22:11 ` [PATCH 4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options' Mel Gorman
@ 2025-01-23 23:51 ` Kees Cook
  2025-02-11  0:45 ` Kees Cook
  5 siblings, 0 replies; 10+ messages in thread
From: Kees Cook @ 2025-01-23 23:51 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Daniel Micay, Paul Moore, linux-hardening, linux-kernel

On Thu, Jan 23, 2025 at 10:11:11PM +0000, Mel Gorman wrote:
> Changelog since v2
> o Default on
> o Logic correction and simplification

Thanks, this look good to me! I'll get this added to the hardening tree
once the merge window closes.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time
  2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
                   ` (4 preceding siblings ...)
  2025-01-23 23:51 ` [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Kees Cook
@ 2025-02-11  0:45 ` Kees Cook
  5 siblings, 0 replies; 10+ messages in thread
From: Kees Cook @ 2025-02-11  0:45 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Kees Cook, Daniel Micay, Paul Moore, linux-hardening,
	linux-kernel

On Thu, 23 Jan 2025 22:11:11 +0000, Mel Gorman wrote:
> Changelog since v2
> o Default on
> o Logic correction and simplification
> 
> Changelog since v1
> o Menu section rename
> o Make static branch usage similar to init_on_alloc
> o Change ordering of menu options
> 
> [...]

Applied to for-next/hardening, thanks!

[1/4] mm: security: Move hardened usercopy under 'Kernel hardening options'
      https://git.kernel.org/kees/c/8907c768bc27
[2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time
      https://git.kernel.org/kees/c/caba7c35e832
[3/4] mm: security: Check early if HARDENED_USERCOPY is enabled
      https://git.kernel.org/kees/c/3d2220040476
[4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options'
      https://git.kernel.org/kees/c/b6518de35d6f

Take care,

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-02-11  0:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-23 22:11 [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
2025-01-23 22:11 ` [PATCH 1/4] mm: security: Move hardened usercopy under 'Kernel hardening options' Mel Gorman
2025-01-23 22:11 ` [PATCH 2/4] mm: security: Allow default HARDENED_USERCOPY to be set at compile time Mel Gorman
2025-01-23 22:11 ` [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled Mel Gorman
2025-01-23 22:11 ` [PATCH 4/4] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options' Mel Gorman
2025-01-23 23:51 ` [PATCH v3 0/4] Allow default HARDENED_USERCOPY to be set at compile time Kees Cook
2025-02-11  0:45 ` Kees Cook
  -- strict thread matches above, loose matches on Subject: below --
2025-01-22 17:19 [PATCH v2 " Mel Gorman
2025-01-22 17:19 ` [PATCH 3/4] mm: security: Check early if HARDENED_USERCOPY is enabled Mel Gorman
2025-01-23  1:01   ` Kees Cook
2025-01-23 11:47     ` Mel Gorman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox