diff mbox series

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

Message ID 20250122171925.25472-3-mgorman@techsingularity.net (mailing list archive)
State Superseded
Headers show
Series Allow default HARDENED_USERCOPY to be set at compile time | expand

Commit Message

Mel Gorman Jan. 22, 2025, 5:19 p.m. UTC
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(-)

Comments

Kees Cook Jan. 23, 2025, 12:57 a.m. UTC | #1
On Wed, Jan 22, 2025 at 05:19:23PM +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);
>  
>  static int __init parse_hardened_usercopy(char *str)
>  {
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 9088d613d519..adcc260839c7 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

This must be "default HARDENED_USERCOPY" or existing distro builds will
break. All major distros enable this by default, and I don't want to
risk HARDENED_USERCOPY_DEFAULT_ON getting missed and getting globally
disabled.

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

-Kees
Mel Gorman Jan. 23, 2025, 11:37 a.m. UTC | #2
On Wed, Jan 22, 2025 at 04:57:37PM -0800, Kees Cook wrote:
> On Wed, Jan 22, 2025 at 05:19:23PM +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);
> >  
> >  static int __init parse_hardened_usercopy(char *str)
> >  {
> > diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> > index 9088d613d519..adcc260839c7 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
> 
> This must be "default HARDENED_USERCOPY" or existing distro builds will
> break. All major distros enable this by default, and I don't want to
> risk HARDENED_USERCOPY_DEFAULT_ON getting missed and getting globally
> disabled.
> 

Ok. I dislike that HARDENED_USERCOPY will be inconsistent with INIT_ON*
but it's not a hill I'm willing to die on. Will be in v3
David Laight Jan. 23, 2025, 9:10 p.m. UTC | #3
...  
> > +config HARDENED_USERCOPY_DEFAULT_ON
> > +	bool "Harden memory copies by default"
> > +	depends on HARDENED_USERCOPY
> > +	default n  
> 
> This must be "default HARDENED_USERCOPY" or existing distro builds will
> break. All major distros enable this by default, and I don't want to
> risk HARDENED_USERCOPY_DEFAULT_ON getting missed and getting globally
> disabled.

It'll also cause grief for anyone trying to bisect.
Although that is always going to go wrong if it has been disabled.

I had 'fun' trying to locate a massive slowdown of a single threaded
program that was caused by a side effect of one of the speculative
execution mitigations getting enabled because the config parameter
got renamed.

	David
diff mbox series

Patch

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..adcc260839c7 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"