[PATCH 0/3] Allow default HARDENED_USERCOPY to be set at compile time

[PATCH 0/3] Allow default HARDENED_USERCOPY to be set at compile time

[Mel Gorman]

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 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 ++-
 mm/usercopy.c                                 |  3 +-
 security/Kconfig                              | 21 ------------
 security/Kconfig.hardening                    | 33 +++++++++++++++++++
 4 files changed, 38 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH 1/3] mm: security: Move hardened usercopy under 'Kernel hardening options'

[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>
---
 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..00e6e2ed0c43 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -279,6 +279,22 @@ config ZERO_CALL_USED_REGS
 
 endmenu
 
+menu "String manipulation"
+
+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

[PATCH 2/3] mm: security: Allow default HARDENED_USERCOPY to be set at compile time

[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 00e6e2ed0c43..537a6431892e 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 n
+	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

[PATCH 3/3] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options'

[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>
---
 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 537a6431892e..8d005fe154ef 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -301,6 +301,15 @@ config HARDENED_USERCOPY_DEFAULT_ON
 	  This has the effect of setting "hardened_usercopy=on" on the kernel
 	  command line. This can be disabled with "hardened_usercopy=off".
 
+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.
+
 endmenu
 
 menu "Hardening of kernel data structures"
-- 
2.43.0

Re: [PATCH 0/3] Allow default HARDENED_USERCOPY to be set at compile time

[Kees Cook]

On Fri, Jan 17, 2025 at 01:03:34PM +0000, Mel Gorman wrote:
> 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.

Yeah! I like this. It's been somewhere on my TODO list for a while, so
thank you for doing it!

Nits/ideas in the patch replies...

-- 
Kees Cook

Re: [PATCH 1/3] mm: security: Move hardened usercopy under 'Kernel hardening options'

[Kees Cook]

On Fri, Jan 17, 2025 at 01:03:35PM +0000, Mel Gorman wrote:
> 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>
> ---
>  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..00e6e2ed0c43 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -279,6 +279,22 @@ config ZERO_CALL_USED_REGS
>  
>  endmenu
>  
> +menu "String manipulation"

I think "string" means different things to different people. I'd prefer
"Bounds checking" or "Spatial safety" if it's going to be a separate
menu section.

> +
> +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"

Otherwise, looks good.

-- 
Kees Cook

Re: [PATCH 2/3] mm: security: Allow default HARDENED_USERCOPY to be set at compile time

[Kees Cook]

On Fri, Jan 17, 2025 at 01:03:36PM +0000, Mel Gorman wrote:
> 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);

With the addition of the compile-time default, we can also provide
better hot-path hinting for the static branches (likely as a separate
patch), that would rename "bypass_usercopy_checks" to
"perform_usercopy_checks" to avoid confusing negatives, and then:

DEFINE_STATIC_KEY_MAYBE_RO(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
			   perform_usercopy_checks);

and then adjust set_hardened_usercopy:

static int __init set_hardened_usercopy(void)
{
        if (enable_checks)
                static_branch_enable(&perform_usercopy_checks);
	else
                static_branch_disable(&perform_usercopy_checks);
        return 1;
}

and finally adjust __check_object_size:

        if (!static_branch_maybe(CONFIG_HARDENED_USERCOPY_DEFAULT_ON,
				 &perform_usercopy_checks))
                return;

But if the perf difference isn't measurable (it's probably lost to the
cost of doing the checks), this change isn't needed at all, but would
fully duplicate the logic used for init_on_alloc etc.

>  
>  static int __init parse_hardened_usercopy(char *str)
>  {
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 00e6e2ed0c43..537a6431892e 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 n

To avoid regressions for people moving their configs forward (and IMO
get the right setting by default), I think this should instead be
"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
> 

Otherwise looks good!

-- 
Kees Cook

Re: [PATCH 3/3] fortify: Move FORTIFY_SOURCE under 'Kernel hardening options'

[Kees Cook]

On Fri, Jan 17, 2025 at 01:03:37PM +0000, Mel Gorman wrote:
> 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>
> ---
>  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 537a6431892e..8d005fe154ef 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -301,6 +301,15 @@ config HARDENED_USERCOPY_DEFAULT_ON
>  	  This has the effect of setting "hardened_usercopy=on" on the kernel
>  	  command line. This can be disabled with "hardened_usercopy=off".
>  
> +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.
> +
>  endmenu

Please move this before HARDENED_USERCOPY -- it's a more general config
and also comes first alphabetically. ;)

I would note that the LLVM bug referenced was fixed in Clang 15+, a
separate patch (not required by you) should fix that if it the tests
actually pass on x86_32 again...

>  
>  menu "Hardening of kernel data structures"
> -- 
> 2.43.0
> 

-Kees

-- 
Kees Cook

Re: [PATCH 1/3] mm: security: Move hardened usercopy under 'Kernel hardening options'

[Paul Moore]

On Fri, Jan 17, 2025 at 8:39 AM Mel Gorman <mgorman@techsingularity.net> wrote:
>
> 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>
> ---
>  security/Kconfig           | 12 ------------
>  security/Kconfig.hardening | 16 ++++++++++++++++
>  2 files changed, 16 insertions(+), 12 deletions(-)

Agree with Kees' comment regarding "Bounds checking" instead of
"String manipulation", but beyond that this is fine with me.

Acked-by: Paul Moore <paul@paul-moore.com>

> 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..00e6e2ed0c43 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -279,6 +279,22 @@ config ZERO_CALL_USED_REGS
>
>  endmenu
>
> +menu "String manipulation"
> +
> +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
>
>


-- 
paul-moore.com

Re: [PATCH 1/3] mm: security: Move hardened usercopy under 'Kernel hardening options'

[Mel Gorman]

On Mon, Jan 20, 2025 at 01:10:44PM -0800, Kees Cook wrote:
> On Fri, Jan 17, 2025 at 01:03:35PM +0000, Mel Gorman wrote:
> > 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>
> > ---
> >  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..00e6e2ed0c43 100644
> > --- a/security/Kconfig.hardening
> > +++ b/security/Kconfig.hardening
> > @@ -279,6 +279,22 @@ config ZERO_CALL_USED_REGS
> >  
> >  endmenu
> >  
> > +menu "String manipulation"
> 
> I think "string" means different things to different people. I'd prefer
> "Bounds checking" or "Spatial safety" if it's going to be a separate
> menu section.
> 

I will change it to "Bounds checking" in v2.

Thanks.

-- 
Mel Gorman
SUSE Labs

Re: [PATCH 2/3] mm: security: Allow default HARDENED_USERCOPY to be set at compile time

[Mel Gorman]

On Mon, Jan 20, 2025 at 01:21:54PM -0800, Kees Cook wrote:
> On Fri, Jan 17, 2025 at 01:03:36PM +0000, Mel Gorman wrote:
> > 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);
> 
> With the addition of the compile-time default, we can also provide
> better hot-path hinting for the static branches (likely as a separate
> patch), that would rename "bypass_usercopy_checks" to
> "perform_usercopy_checks" to avoid confusing negatives, and then:
> 

I decided to go with the more explicit name "validate_usercopy_range"
but I'm not overly pushed about the name

> > --- 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 n
>
> To avoid regressions for people moving their configs forward (and IMO
> get the right setting by default), I think this should instead be
> "default HARDENED_USERCOPY".

This I'm less keen on. The intent is that all the hardening options would
default to all opt-in or all opt-out by default to be consistent. I lean
towards default opt-in because those requiring a hardened environment should
know what options to enable and why. A distribution providing a hardened
kernel would explicitly enable them by default. A distribution that wanted
to provided a general kernel for users could provide the hardening options
but leave them disabled to avoid spurious performance regression reports
after a kernel upgrade.

Micro-optimisation currently is this;

--<--
mm: security: Check early if HARDENED_USERCOPY is enabled

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

Suggested-by: Kees Cook <kees@kernel.org>
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>

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;
 }