[PATCH v3] debugfs: Add access restriction option

[PATCH v3] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

When enabled it is needed a kernel command line parameter to be activated.

It can be on or off, but also internally on but not seen from user-space.
This no-fs mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.

 .../admin-guide/kernel-parameters.txt         | 11 +++++
 fs/debugfs/inode.c                            | 47 +++++++++++++++++++
 lib/Kconfig.debug                             | 10 ++++
 3 files changed, 68 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..249c86e53bb7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,17 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] When CONFIG_DEBUG_FS_RESTRICTED is set, this parameter
+			enables what is exposed to userspace.
+			Format: { on, no_fs, off }
+			on: 	All functions are enabled.
+			no_fs: 	Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				it's content. There its nothing to mount.
+			off: 	(default) Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..2bd80a932ae1 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -31,10 +31,17 @@
 #include "internal.h"
 
 #define DEBUGFS_DEFAULT_MODE	0700
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+#define DEBUGFS_ALLOW_API 0x2
+#define DEBUGFS_ALLOW_FS 0x1
+#endif
 
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+static unsigned int debugfs_allow;
+#endif
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +273,10 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+#endif
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -385,6 +396,12 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+#endif
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +558,12 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+#endif
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +606,12 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+#endif
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +815,28 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (str && !strcmp(str, "on"))
+		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_FS;
+	if (str && !strcmp(str, "no-fs"))
+		debugfs_allow |= DEBUGFS_ALLOW_API;
+	if (str && !strcmp(str, "off"))
+		debugfs_allow = 0;
+#endif
+	return 0;
+
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+#ifdef CONFIG_DEBUG_FS_RESTRICTED
+	if (!(debugfs_allow & DEBUGFS_ALLOW_FS))
+		return -EPERM;
+#endif
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d74ac0fd6b2d..19fdaae14e36 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -477,6 +477,16 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+config DEBUG_FS_RESTRICTED
+	bool "Debug Filesystem restricted"
+	depends on DEBUG_FS
+	help
+	  This is an additional restriction for mounting debugfs. It allows
+	  the kernel to have debugfs compiled, but requires that kernel command
+	  line has a debugfs parameter to register as a filesystem.
+
+	  If unsure, say N.
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.17.1

Re: [PATCH v3] debugfs: Add access restriction option

[Greg Kroah-Hartman]

On Wed, Jun 17, 2020 at 03:37:38PM +0200, Peter Enderborg wrote:
> Since debugfs include sensitive information it need to be treated
> carefully. But it also has many very useful debug functions for userspace.
> With this option we can have same configuration for system with
> need of debugfs and a way to turn it off. This gives a extra protection
> for exposure on systems where user-space services with system
> access are attacked.
> 
> When enabled it is needed a kernel command line parameter to be activated.
> 
> It can be on or off, but also internally on but not seen from user-space.
> This no-fs mode do not register a debugfs as filesystem, but client can
> register their parts in the internal structures. This data can be readed
> with a debugger or saved with a crashkernel. When it is off clients
> get EPERM error when accessing the functions for registering their
> components.
> 
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
> v2. Removed MOUNT as part of restrictions. Added API's restrictions as
>     separate restriction.
> v3  Updated Documentation after Randy Dunlap reviews and suggestions.
> 
>  .../admin-guide/kernel-parameters.txt         | 11 +++++
>  fs/debugfs/inode.c                            | 47 +++++++++++++++++++
>  lib/Kconfig.debug                             | 10 ++++
>  3 files changed, 68 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fb95fad81c79..249c86e53bb7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -827,6 +827,17 @@
>  			useful to also enable the page_owner functionality.
>  			on: enable the feature
>  
> +	debugfs=    	[KNL] When CONFIG_DEBUG_FS_RESTRICTED is set, this parameter
> +			enables what is exposed to userspace.
> +			Format: { on, no_fs, off }
> +			on: 	All functions are enabled.
> +			no_fs: 	Filesystem is not registered but kernel clients can
> +			        access APIs and a crashkernel can be used to read
> +				it's content. There its nothing to mount.
> +			off: 	(default) Filesystem is not registered and clients
> +			        get a -EPERM as result when trying to register files
> +				or directories within debugfs.
> +
>  	debugpat	[X86] Enable PAT debugging
>  
>  	decnet.addr=	[HW,NET]
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index b7f2e971ecbc..2bd80a932ae1 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -31,10 +31,17 @@
>  #include "internal.h"
>  
>  #define DEBUGFS_DEFAULT_MODE	0700
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +#define DEBUGFS_ALLOW_API 0x2
> +#define DEBUGFS_ALLOW_FS 0x1

BIT()?

And a tab?

And why a #ifdef?

> +#endif
>  
>  static struct vfsmount *debugfs_mount;
>  static int debugfs_mount_count;
>  static bool debugfs_registered;
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +static unsigned int debugfs_allow;
> +#endif

Why #ifdef?

>  
>  /*
>   * Don't allow access attributes to be changed whilst the kernel is locked down
> @@ -266,6 +273,10 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
>  			int flags, const char *dev_name,
>  			void *data)
>  {
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
> +		return ERR_PTR(-EPERM);
> +#endif

Ick, all of this #ifdef is a mess, and can be totally avoided if you do
the logic right here.  Please make it so that the functions and almost
all of the .c code does not have #ifdef CONFIG_DEBUG_FS_RESTRICTED at
all.


>  	return mount_single(fs_type, flags, data, debug_fill_super);
>  }
>  
> @@ -385,6 +396,12 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +#endif
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create file '%s'\n",
> @@ -541,6 +558,12 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +#endif
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create directory '%s'\n",
> @@ -583,6 +606,12 @@ struct dentry *debugfs_create_automount(const char *name,
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +#endif
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create automount '%s'\n",
> @@ -786,10 +815,28 @@ bool debugfs_initialized(void)
>  }
>  EXPORT_SYMBOL_GPL(debugfs_initialized);
>  
> +static int __init debugfs_kernel(char *str)
> +{
> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
> +	if (str && !strcmp(str, "on"))
> +		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_FS;
> +	if (str && !strcmp(str, "no-fs"))
> +		debugfs_allow |= DEBUGFS_ALLOW_API;
> +	if (str && !strcmp(str, "off"))
> +		debugfs_allow = 0;

It's set to 0 by default, no need to set it again, right?

> +#endif
> +	return 0;
> +
> +}
> +early_param("debugfs", debugfs_kernel);

Why is this a valid parm even if the option is not enabled?  Do you mean
to do that?  Why?

thanks,

greg k-h

Re: [PATCH v3] debugfs: Add access restriction option

[Enderborg, Peter]

On 6/17/20 4:15 PM, Greg Kroah-Hartman wrote:
> On Wed, Jun 17, 2020 at 03:37:38PM +0200, Peter Enderborg wrote:
>> Since debugfs include sensitive information it need to be treated
>> carefully. But it also has many very useful debug functions for userspace.
>> With this option we can have same configuration for system with
>> need of debugfs and a way to turn it off. This gives a extra protection
>> for exposure on systems where user-space services with system
>> access are attacked.
>>
>> When enabled it is needed a kernel command line parameter to be activated.
>>
>> It can be on or off, but also internally on but not seen from user-space.
>> This no-fs mode do not register a debugfs as filesystem, but client can
>> register their parts in the internal structures. This data can be readed
>> with a debugger or saved with a crashkernel. When it is off clients
>> get EPERM error when accessing the functions for registering their
>> components.
>>
>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
>> ---
>> v2. Removed MOUNT as part of restrictions. Added API's restrictions as
>>     separate restriction.
>> v3  Updated Documentation after Randy Dunlap reviews and suggestions.
>>
>>  .../admin-guide/kernel-parameters.txt         | 11 +++++
>>  fs/debugfs/inode.c                            | 47 +++++++++++++++++++
>>  lib/Kconfig.debug                             | 10 ++++
>>  3 files changed, 68 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index fb95fad81c79..249c86e53bb7 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -827,6 +827,17 @@
>>  			useful to also enable the page_owner functionality.
>>  			on: enable the feature
>>  
>> +	debugfs=    	[KNL] When CONFIG_DEBUG_FS_RESTRICTED is set, this parameter
>> +			enables what is exposed to userspace.
>> +			Format: { on, no_fs, off }
>> +			on: 	All functions are enabled.
>> +			no_fs: 	Filesystem is not registered but kernel clients can
>> +			        access APIs and a crashkernel can be used to read
>> +				it's content. There its nothing to mount.
>> +			off: 	(default) Filesystem is not registered and clients
>> +			        get a -EPERM as result when trying to register files
>> +				or directories within debugfs.
>> +
>>  	debugpat	[X86] Enable PAT debugging
>>  
>>  	decnet.addr=	[HW,NET]
>> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
>> index b7f2e971ecbc..2bd80a932ae1 100644
>> --- a/fs/debugfs/inode.c
>> +++ b/fs/debugfs/inode.c
>> @@ -31,10 +31,17 @@
>>  #include "internal.h"
>>  
>>  #define DEBUGFS_DEFAULT_MODE	0700
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +#define DEBUGFS_ALLOW_API 0x2
>> +#define DEBUGFS_ALLOW_FS 0x1
> BIT()?
>
> And a tab?
>
> And why a #ifdef?
To get it as least intrusive as possible. A solid Opt-In.
>
>> +#endif
>>  
>>  static struct vfsmount *debugfs_mount;
>>  static int debugfs_mount_count;
>>  static bool debugfs_registered;
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +static unsigned int debugfs_allow;
>> +#endif
> Why #ifdef?
>
>>  
>>  /*
>>   * Don't allow access attributes to be changed whilst the kernel is locked down
>> @@ -266,6 +273,10 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
>>  			int flags, const char *dev_name,
>>  			void *data)
>>  {
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
>> +		return ERR_PTR(-EPERM);
>> +#endif
> Ick, all of this #ifdef is a mess, and can be totally avoided if you do
> the logic right here.  Please make it so that the functions and almost
> all of the .c code does not have #ifdef CONFIG_DEBUG_FS_RESTRICTED at
> all.
>
Is it ok to remove the #ifdefs and let code always be there and let the config set the default value?


>>  	return mount_single(fs_type, flags, data, debug_fill_super);
>>  }
>>  
>> @@ -385,6 +396,12 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
>>  	if (IS_ERR(dentry))
>>  		return dentry;
>>  
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
>> +		failed_creating(dentry);
>> +		return ERR_PTR(-EPERM);
>> +	}
>> +#endif
>>  	inode = debugfs_get_inode(dentry->d_sb);
>>  	if (unlikely(!inode)) {
>>  		pr_err("out of free dentries, can not create file '%s'\n",
>> @@ -541,6 +558,12 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
>>  	if (IS_ERR(dentry))
>>  		return dentry;
>>  
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
>> +		failed_creating(dentry);
>> +		return ERR_PTR(-EPERM);
>> +	}
>> +#endif
>>  	inode = debugfs_get_inode(dentry->d_sb);
>>  	if (unlikely(!inode)) {
>>  		pr_err("out of free dentries, can not create directory '%s'\n",
>> @@ -583,6 +606,12 @@ struct dentry *debugfs_create_automount(const char *name,
>>  	if (IS_ERR(dentry))
>>  		return dentry;
>>  
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
>> +		failed_creating(dentry);
>> +		return ERR_PTR(-EPERM);
>> +	}
>> +#endif
>>  	inode = debugfs_get_inode(dentry->d_sb);
>>  	if (unlikely(!inode)) {
>>  		pr_err("out of free dentries, can not create automount '%s'\n",
>> @@ -786,10 +815,28 @@ bool debugfs_initialized(void)
>>  }
>>  EXPORT_SYMBOL_GPL(debugfs_initialized);
>>  
>> +static int __init debugfs_kernel(char *str)
>> +{
>> +#ifdef CONFIG_DEBUG_FS_RESTRICTED
>> +	if (str && !strcmp(str, "on"))
>> +		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_FS;
>> +	if (str && !strcmp(str, "no-fs"))
>> +		debugfs_allow |= DEBUGFS_ALLOW_API;
>> +	if (str && !strcmp(str, "off"))
>> +		debugfs_allow = 0;
> It's set to 0 by default, no need to set it again, right?
I think there have been some issues with the same parameter more than once.
>
>> +#endif
>> +	return 0;
>> +
>> +}
>> +early_param("debugfs", debugfs_kernel);
> Why is this a valid parm even if the option is not enabled?  Do you mean
> to do that?  Why?

I did not find any good usage where it was config dependent, when it is there; it is  "reserve" the name.

It will always be there if we remove the #ifdefs.

> thanks,
>
> greg k-h

Re: [PATCH v3] debugfs: Add access restriction option

[Randy Dunlap]

On 6/17/20 6:37 AM, Peter Enderborg wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fb95fad81c79..249c86e53bb7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -827,6 +827,17 @@
>  			useful to also enable the page_owner functionality.
>  			on: enable the feature
>  
> +	debugfs=    	[KNL] When CONFIG_DEBUG_FS_RESTRICTED is set, this parameter
> +			enables what is exposed to userspace.
> +			Format: { on, no_fs, off }
> +			on: 	All functions are enabled.
> +			no_fs: 	Filesystem is not registered but kernel clients can
> +			        access APIs and a crashkernel can be used to read
> +				it's content. There its nothing to mount.

				its content. There is nothing to mount.


> +			off: 	(default) Filesystem is not registered and clients
> +			        get a -EPERM as result when trying to register files
> +				or directories within debugfs.
> +
>  	debugpat	[X86] Enable PAT debugging
>  
>  	decnet.addr=	[HW,NET]


-- 
~Randy

[PATCH v4 0/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.
v4  Removed #ifdefs from inode.c and using internal.h for configuration
    and now using BIT() for that. Function is now always on, and are
    instead selected by a built in default or command line parameter.
    Added preparation patch that removes check debugfs is initialised.
    Reported-by: kernel test robot <lkp@intel.com>

[PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Peter Enderborg]

This is a preparation for debugfs restricted mode.
We don't need debugfs to trace, the removed check stop tracefs to work
if debugfs is not initialised. We instead tries to automount within
debugfs and relay on it's handling. The code path is to create a
backward compatibility from when tracefs was part of debugfs, it is now
standalone and does not need debugfs. When debugfs is in restricted
it is compiled in but not active and return EPERM to clients and
tracefs wont work if it assumes it is active it is compiled in
kernel.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 kernel/trace/trace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ec44b0e2a19c..34ed82364edb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8946,9 +8946,7 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!tracefs_initialized()) ||
-		(IS_ENABLED(CONFIG_DEBUG_FS) &&
-		 WARN_ON(!debugfs_initialized())))
+	if (WARN_ON(!tracefs_initialized()))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.26.2

Re: [PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Greg Kroah-Hartman]

On Mon, Jun 22, 2020 at 10:30:18AM +0200, Peter Enderborg wrote:
> This is a preparation for debugfs restricted mode.
> We don't need debugfs to trace, the removed check stop tracefs to work
> if debugfs is not initialised. We instead tries to automount within
> debugfs and relay on it's handling. The code path is to create a
> backward compatibility from when tracefs was part of debugfs, it is now
> standalone and does not need debugfs. When debugfs is in restricted
> it is compiled in but not active and return EPERM to clients and
> tracefs wont work if it assumes it is active it is compiled in
> kernel.

I'm sorry, but I can't parse this changelog text at all.  Why exactly
are you doing this?

> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  kernel/trace/trace.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ec44b0e2a19c..34ed82364edb 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -8946,9 +8946,7 @@ struct dentry *tracing_init_dentry(void)
>  	if (tr->dir)
>  		return NULL;
>  
> -	if (WARN_ON(!tracefs_initialized()) ||
> -		(IS_ENABLED(CONFIG_DEBUG_FS) &&
> -		 WARN_ON(!debugfs_initialized())))
> +	if (WARN_ON(!tracefs_initialized()))
>  		return ERR_PTR(-ENODEV);
>  

This change makes sense to me anyway, so:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Steven Rostedt]

On Mon, 22 Jun 2020 10:30:18 +0200
Peter Enderborg <peter.enderborg@sony.com> wrote:

> This is a preparation for debugfs restricted mode.
> We don't need debugfs to trace, the removed check stop tracefs to work
> if debugfs is not initialised. We instead tries to automount within
> debugfs and relay on it's handling. The code path is to create a
> backward compatibility from when tracefs was part of debugfs, it is now
> standalone and does not need debugfs. When debugfs is in restricted
> it is compiled in but not active and return EPERM to clients and
> tracefs wont work if it assumes it is active it is compiled in
> kernel.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> ---
>  kernel/trace/trace.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ec44b0e2a19c..34ed82364edb 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -8946,9 +8946,7 @@ struct dentry *tracing_init_dentry(void)
>  	if (tr->dir)
>  		return NULL;
>  
> -	if (WARN_ON(!tracefs_initialized()) ||
> -		(IS_ENABLED(CONFIG_DEBUG_FS) &&
> -		 WARN_ON(!debugfs_initialized())))
> +	if (WARN_ON(!tracefs_initialized()))
>  		return ERR_PTR(-ENODEV);
>  
>  	/*

[PATCH 2/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

It is controlled by a configurable default value that can be override
with a kernel command line parameter. (debugfs=)

It can be on or off, but also internally on but not seen from user-space.
This no-fs mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 12 ++++++
 fs/debugfs/inode.c                            | 37 +++++++++++++++++++
 fs/debugfs/internal.h                         | 14 +++++++
 lib/Kconfig.debug                             | 32 ++++++++++++++++
 4 files changed, 95 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..236aacaceaf5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,18 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] This parameter enables what is exposed to userspace
+			and debugfs internal clients.
+			Format: { on, no-fs, off }
+			on: 	All functions are enabled.
+			no-fs: 	Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				its content. There is nothing to mount.
+			off: 	Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+			Default value is set in build-time with a kernel configuration.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..a4a1c92ae478 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -35,6 +35,7 @@
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ACCESS_BITS;
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT))
+		return ERR_CAST(ERR_PTR(-EPERM));
+
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
 	struct dentry *dentry;
 	int error;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT))
+		return ERR_PTR(-EPERM);
+
 	pr_debug("creating file '%s'\n", name);
 
 	if (IS_ERR(parent))
@@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +808,25 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+	if (str && !strcmp(str, "on"))
+		debugfs_allow = DEBUGFS_ALLOW_API_BIT | DEBUGFS_ALLOW_FS_BIT;
+	if (str && !strcmp(str, "no-fs"))
+		debugfs_allow = DEBUGFS_ALLOW_API_BIT;
+	if (str && !strcmp(str, "off"))
+		debugfs_allow = 0;
+
+	return 0;
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_FS_BIT))
+		return -EPERM;
+
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 034e6973cead..dba138f8d418 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -29,4 +29,18 @@ struct debugfs_fsdata {
  */
 #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
 
+/* Access BITS */
+#define DEBUGFS_ALLOW_API_BIT BIT(0)
+#define DEBUGFS_ALLOW_FS_BIT BIT(1)
+
+#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
+#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS_BIT|DEBUGFS_ALLOW_API_BIT)
+#endif
+#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
+#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API_BIT)
+#endif
+#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
+#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
+#endif
+
 #endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d74ac0fd6b2d..4c699ffad1fb 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -477,6 +477,38 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+choice
+	prompt "Debugfs default access"
+	depends on DEBUG_FS
+	default DEBUG_FS_ACCESS_ALL
+	help
+	  This select the default access restricions for debugfs.
+	  It can be overridden with kernel command line option
+	  debugfs=[on,no-fs,off] The restrictions apply for API access
+	  and filesystem registration. .
+
+config DEBUG_FS_ACCESS_ALL
+       bool "Access all"
+       help
+	  No restrictions applies. Both API and filesystem registration
+	  is on.
+
+config DEBUG_FS_ACCESS_NO_FS
+       bool "Do not register debugfs as filesystem"
+       help
+	 The API is open but filesystem not loaded. Client can still do
+	 their work and readed with debug tools that does not need
+	 debugfs filesystem.
+
+config DEBUG_FS_ACCESS_NONE
+       bool "No access"
+       help
+	  Access is off. Clients get EPERM when trying to create nodes in
+	  debugfs tree and debugfs is not registred as an filesystem.
+	  Client can then back-off or continue without debugfs access.
+
+endchoice
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.26.2

Re: [PATCH 2/2] debugfs: Add access restriction option

[Greg Kroah-Hartman]

On Mon, Jun 22, 2020 at 10:30:19AM +0200, Peter Enderborg wrote:
> Since debugfs include sensitive information it need to be treated
> carefully. But it also has many very useful debug functions for userspace.
> With this option we can have same configuration for system with
> need of debugfs and a way to turn it off. This gives a extra protection
> for exposure on systems where user-space services with system
> access are attacked.
> 
> It is controlled by a configurable default value that can be override
> with a kernel command line parameter. (debugfs=)
> 
> It can be on or off, but also internally on but not seen from user-space.
> This no-fs mode do not register a debugfs as filesystem, but client can
> register their parts in the internal structures. This data can be readed
> with a debugger or saved with a crashkernel. When it is off clients
> get EPERM error when accessing the functions for registering their
> components.
> 
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 12 ++++++
>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>  fs/debugfs/internal.h                         | 14 +++++++
>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>  4 files changed, 95 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fb95fad81c79..236aacaceaf5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -827,6 +827,18 @@
>  			useful to also enable the page_owner functionality.
>  			on: enable the feature
>  
> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
> +			and debugfs internal clients.
> +			Format: { on, no-fs, off }
> +			on: 	All functions are enabled.
> +			no-fs: 	Filesystem is not registered but kernel clients can
> +			        access APIs and a crashkernel can be used to read
> +				its content. There is nothing to mount.
> +			off: 	Filesystem is not registered and clients
> +			        get a -EPERM as result when trying to register files
> +				or directories within debugfs.

Can you add "This is equilivant of the runtime functionality if debugfs
was not enabled in the kernel at all." to the "off" option?



> +			Default value is set in build-time with a kernel configuration.
> +
>  	debugpat	[X86] Enable PAT debugging
>  
>  	decnet.addr=	[HW,NET]
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index b7f2e971ecbc..a4a1c92ae478 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -35,6 +35,7 @@
>  static struct vfsmount *debugfs_mount;
>  static int debugfs_mount_count;
>  static bool debugfs_registered;
> +static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ACCESS_BITS;
>  
>  /*
>   * Don't allow access attributes to be changed whilst the kernel is locked down
> @@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
>  			int flags, const char *dev_name,
>  			void *data)
>  {
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT))
> +		return ERR_CAST(ERR_PTR(-EPERM));

Shouldn't ERR_PTR() be all that is needed here?  I don't see any
ERR_CAST() usages in this format in the kernel today.

> +
>  	return mount_single(fs_type, flags, data, debug_fill_super);
>  }
>  
> @@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
>  	struct dentry *dentry;
>  	int error;
>  
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT))
> +		return ERR_PTR(-EPERM);

See, you don't use it here :)



> +
>  	pr_debug("creating file '%s'\n", name);
>  
>  	if (IS_ERR(parent))
> @@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create file '%s'\n",
> @@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create directory '%s'\n",
> @@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
>  	if (IS_ERR(dentry))
>  		return dentry;
>  
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_API_BIT)) {
> +		failed_creating(dentry);
> +		return ERR_PTR(-EPERM);
> +	}
> +
>  	inode = debugfs_get_inode(dentry->d_sb);
>  	if (unlikely(!inode)) {
>  		pr_err("out of free dentries, can not create automount '%s'\n",
> @@ -786,10 +808,25 @@ bool debugfs_initialized(void)
>  }
>  EXPORT_SYMBOL_GPL(debugfs_initialized);
>  
> +static int __init debugfs_kernel(char *str)
> +{
> +	if (str && !strcmp(str, "on"))
> +		debugfs_allow = DEBUGFS_ALLOW_API_BIT | DEBUGFS_ALLOW_FS_BIT;
> +	if (str && !strcmp(str, "no-fs"))
> +		debugfs_allow = DEBUGFS_ALLOW_API_BIT;
> +	if (str && !strcmp(str, "off"))
> +		debugfs_allow = 0;
> +
> +	return 0;
> +}
> +early_param("debugfs", debugfs_kernel);
>  static int __init debugfs_init(void)
>  {
>  	int retval;
>  
> +	if (!(debugfs_allow & DEBUGFS_ALLOW_FS_BIT))
> +		return -EPERM;
> +
>  	retval = sysfs_create_mount_point(kernel_kobj, "debug");
>  	if (retval)
>  		return retval;
> diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
> index 034e6973cead..dba138f8d418 100644
> --- a/fs/debugfs/internal.h
> +++ b/fs/debugfs/internal.h
> @@ -29,4 +29,18 @@ struct debugfs_fsdata {
>   */
>  #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
>  
> +/* Access BITS */
> +#define DEBUGFS_ALLOW_API_BIT BIT(0)
> +#define DEBUGFS_ALLOW_FS_BIT BIT(1)

tabs please.

And no need for _BIT in the string, right?

> +
> +#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS_BIT|DEBUGFS_ALLOW_API_BIT)

' ' is your friend :)

> +#endif
> +#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API_BIT)
> +#endif
> +#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
> +#endif
> +
>  #endif /* _DEBUGFS_INTERNAL_H_ */
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d74ac0fd6b2d..4c699ffad1fb 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -477,6 +477,38 @@ config DEBUG_FS
>  
>  	  If unsure, say N.
>  
> +choice
> +	prompt "Debugfs default access"
> +	depends on DEBUG_FS
> +	default DEBUG_FS_ACCESS_ALL
> +	help
> +	  This select the default access restricions for debugfs.
> +	  It can be overridden with kernel command line option
> +	  debugfs=[on,no-fs,off] The restrictions apply for API access
> +	  and filesystem registration. .
> +
> +config DEBUG_FS_ACCESS_ALL
> +       bool "Access all"

"Normal access" as this is what we have today, "Access all" doesn't
really explain what this is.

> +       help
> +	  No restrictions applies. Both API and filesystem registration
> +	  is on.

Add:
	"This is the normal default operation"

Or something like that?

And you mix tabs and spaces a bunch in this patch for this file, did
checkpatch not complain?

thanks,

greg k-h

[PATCH v5 0/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.
v4  Removed #ifdefs from inode.c and using internal.h for configuration
    and now using BIT() for that. Function is now always on, and are
    instead selected by a built in default or command line parameter.
    Changed return value on debug_mount
    Reported-by: kernel test robot <lkp@intel.com>
    Im not sure about that it is right
v5  Added notes to config help suggested by GregKH.
    Removed _BIT from names, white-space and tab.
    (checkpatch did not complain).

[PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Peter Enderborg]

This is a preparation for debugfs restricted mode.
We don't need debugfs to trace, the removed check stop tracefs to work
if debugfs is not initialised. We instead tries to automount within
debugfs and relay on it's handling. The code path is to create a
backward compatibility from when tracefs was part of debugfs, it is now
standalone and does not need debugfs. When debugfs is in restricted
it is compiled in but not active and return EPERM to clients and
tracefs wont work if it assumes it is active it is compiled in
kernel.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..848f67a5f16d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8945,9 +8945,7 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!tracefs_initialized()) ||
-		(IS_ENABLED(CONFIG_DEBUG_FS) &&
-		 WARN_ON(!debugfs_initialized())))
+	if (WARN_ON(!tracefs_initialized()))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.17.1

[PATCH 2/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

It is controlled by a configurable default value that can be override
with a kernel command line parameter. (debugfs=)

It can be on or off, but also internally on but not seen from user-space.
This no-fs mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 14 +++++++
 fs/debugfs/inode.c                            | 37 +++++++++++++++++++
 fs/debugfs/internal.h                         | 14 +++++++
 lib/Kconfig.debug                             | 32 ++++++++++++++++
 4 files changed, 97 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..805aa2e58491 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,20 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] This parameter enables what is exposed to userspace
+			and debugfs internal clients.
+			Format: { on, no-fs, off }
+			on: 	All functions are enabled.
+			no-fs: 	Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				its content. There is nothing to mount.
+			off: 	Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+				This is equilivant of the runtime functionality if
+				debugfs was not enabled in the kernel at all.
+			Default value is set in build-time with a kernel configuration.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..eee937c49a80 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -35,6 +35,7 @@
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ACCESS_BITS;
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
 	struct dentry *dentry;
 	int error;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	pr_debug("creating file '%s'\n", name);
 
 	if (IS_ERR(parent))
@@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +808,25 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+	if (str && !strcmp(str, "on"))
+		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_FS;
+	if (str && !strcmp(str, "no-fs"))
+		debugfs_allow = DEBUGFS_ALLOW_API;
+	if (str && !strcmp(str, "off"))
+		debugfs_allow = 0;
+
+	return 0;
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_FS))
+		return -EPERM;
+
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 034e6973cead..9cd3be76359a 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -29,4 +29,18 @@ struct debugfs_fsdata {
  */
 #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
 
+/* Access BITS */
+#define DEBUGFS_ALLOW_API	BIT(0)
+#define DEBUGFS_ALLOW_FS	BIT(1)
+
+#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
+#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS | DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
+#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
+#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
+#endif
+
 #endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..b609ad7c1343 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -476,6 +476,38 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+choice
+	prompt "Debugfs default access"
+	depends on DEBUG_FS
+	default DEBUG_FS_ACCESS_ALL
+	help
+	  This select the default access restricions for debugfs.
+	  It can be overridden with kernel command line option
+	  debugfs=[on,no-fs,off] The restrictions apply for API access
+	  and filesystem registration. .
+
+config DEBUG_FS_ACCESS_ALL
+       bool "Access normal"
+       help
+	  No restrictions applies. Both API and filesystem registration
+	  is on. This is the normal default operation.
+
+config DEBUG_FS_ACCESS_NO_FS
+       bool "Do not register debugfs as filesystem"
+       help
+	 The API is open but filesystem not loaded. Client can still do
+	 their work and readed with debug tools that does not need
+	 debugfs filesystem.
+
+config DEBUG_FS_ACCESS_NONE
+       bool "No access"
+       help
+	  Access is off. Clients get EPERM when trying to create nodes in
+	  debugfs tree and debugfs is not registred as an filesystem.
+	  Client can then back-off or continue without debugfs access.
+
+endchoice
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.17.1

Re: [PATCH 2/2] debugfs: Add access restriction option

[Greg Kroah-Hartman]

On Wed, Jul 15, 2020 at 10:42:07AM +0200, Peter Enderborg wrote:
> Since debugfs include sensitive information it need to be treated
> carefully. But it also has many very useful debug functions for userspace.
> With this option we can have same configuration for system with
> need of debugfs and a way to turn it off. This gives a extra protection
> for exposure on systems where user-space services with system
> access are attacked.
> 
> It is controlled by a configurable default value that can be override
> with a kernel command line parameter. (debugfs=)
> 
> It can be on or off, but also internally on but not seen from user-space.
> This no-fs mode do not register a debugfs as filesystem, but client can
> register their parts in the internal structures. This data can be readed
> with a debugger or saved with a crashkernel. When it is off clients
> get EPERM error when accessing the functions for registering their
> components.
> 
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 14 +++++++
>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>  fs/debugfs/internal.h                         | 14 +++++++
>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>  4 files changed, 97 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fb95fad81c79..805aa2e58491 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -827,6 +827,20 @@
>  			useful to also enable the page_owner functionality.
>  			on: enable the feature
>  
> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
> +			and debugfs internal clients.
> +			Format: { on, no-fs, off }
> +			on: 	All functions are enabled.
> +			no-fs: 	Filesystem is not registered but kernel clients can
> +			        access APIs and a crashkernel can be used to read
> +				its content. There is nothing to mount.
> +			off: 	Filesystem is not registered and clients
> +			        get a -EPERM as result when trying to register files
> +				or directories within debugfs.
> +				This is equilivant of the runtime functionality if
> +				debugfs was not enabled in the kernel at all.
> +			Default value is set in build-time with a kernel configuration.

Naming is hard.  In looking at this, should "no-fs" be called
"no-mount"?  That's a better description of what it does, right?

Then we can rename the bits to match the documentation so we aren't
constantly thinking of different things and trying to match them up:


> --- a/fs/debugfs/internal.h
> +++ b/fs/debugfs/internal.h
> @@ -29,4 +29,18 @@ struct debugfs_fsdata {
>   */
>  #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
>  
> +/* Access BITS */
> +#define DEBUGFS_ALLOW_API	BIT(0)
> +#define DEBUGFS_ALLOW_FS	BIT(1)

#define DEBUGFS_ALLOW_API	BIT(0)
#define DEBUGFS_ALLOW_MOUNT	BIT(1)

> +#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS | DEBUGFS_ALLOW_API)
> +#endif
> +#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API)
> +#endif
> +#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
> +#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
> +#endif
> +
>  #endif /* _DEBUGFS_INTERNAL_H_ */
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9ad9210d70a1..b609ad7c1343 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -476,6 +476,38 @@ config DEBUG_FS
>  
>  	  If unsure, say N.
>  
> +choice
> +	prompt "Debugfs default access"
> +	depends on DEBUG_FS
> +	default DEBUG_FS_ACCESS_ALL

default DEBUGFS_FS_ALLOW_ALL

> +	help
> +	  This select the default access restricions for debugfs.
> +	  It can be overridden with kernel command line option
> +	  debugfs=[on,no-fs,off] The restrictions apply for API access
> +	  and filesystem registration. .
> +
> +config DEBUG_FS_ACCESS_ALL

config DEBUG_FS_ALLOW_ALL

> +       bool "Access normal"
> +       help
> +	  No restrictions applies. Both API and filesystem registration
> +	  is on. This is the normal default operation.
> +
> +config DEBUG_FS_ACCESS_NO_FS

config DEBUG_FS_DISALLOW_MOUNT

> +       bool "Do not register debugfs as filesystem"
> +       help
> +	 The API is open but filesystem not loaded. Client can still do
> +	 their work and readed with debug tools that does not need
> +	 debugfs filesystem.
> +
> +config DEBUG_FS_ACCESS_NONE

config DEBUG_FS_ALLOW_NONE

Does that sound better?

I'm not trying to be a pain, just trying to name this all correctly as
it's messy with different config options and bits and mapping that to
checks in the code without everything called the same.

thanks,

greg k-h

Re: [PATCH 2/2] debugfs: Add access restriction option

[Enderborg, Peter]

On 7/15/20 11:39 AM, Greg Kroah-Hartman wrote:
> On Wed, Jul 15, 2020 at 10:42:07AM +0200, Peter Enderborg wrote:
>> Since debugfs include sensitive information it need to be treated
>> carefully. But it also has many very useful debug functions for userspace.
>> With this option we can have same configuration for system with
>> need of debugfs and a way to turn it off. This gives a extra protection
>> for exposure on systems where user-space services with system
>> access are attacked.
>>
>> It is controlled by a configurable default value that can be override
>> with a kernel command line parameter. (debugfs=)
>>
>> It can be on or off, but also internally on but not seen from user-space.
>> This no-fs mode do not register a debugfs as filesystem, but client can
>> register their parts in the internal structures. This data can be readed
>> with a debugger or saved with a crashkernel. When it is off clients
>> get EPERM error when accessing the functions for registering their
>> components.
>>
>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
>> ---
>>  .../admin-guide/kernel-parameters.txt         | 14 +++++++
>>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>>  fs/debugfs/internal.h                         | 14 +++++++
>>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>>  4 files changed, 97 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index fb95fad81c79..805aa2e58491 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -827,6 +827,20 @@
>>  			useful to also enable the page_owner functionality.
>>  			on: enable the feature
>>  
>> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
>> +			and debugfs internal clients.
>> +			Format: { on, no-fs, off }
>> +			on: 	All functions are enabled.
>> +			no-fs: 	Filesystem is not registered but kernel clients can
>> +			        access APIs and a crashkernel can be used to read
>> +				its content. There is nothing to mount.
>> +			off: 	Filesystem is not registered and clients
>> +			        get a -EPERM as result when trying to register files
>> +				or directories within debugfs.
>> +				This is equilivant of the runtime functionality if
>> +				debugfs was not enabled in the kernel at all.
>> +			Default value is set in build-time with a kernel configuration.
> Naming is hard.  In looking at this, should "no-fs" be called
> "no-mount"?  That's a better description of what it does, right?

I think no-fs cover it better since it does not register a filesystem
but provides the interfaces. Mounting is then indirectly stopped.

The idea start with a check for mounting but it is much more
definitely stopped by prevention of register of the filesystem.
I can imagine to have a forth "mode" where it register a fs but
not allowing mounting. Such mode maybe useful for some runtime
configuration. But this patch is about boot time configuration.

> Then we can rename the bits to match the documentation so we aren't
> constantly thinking of different things and trying to match them up:
>
>
>> --- a/fs/debugfs/internal.h
>> +++ b/fs/debugfs/internal.h
>> @@ -29,4 +29,18 @@ struct debugfs_fsdata {
>>   */
>>  #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
>>  
>> +/* Access BITS */
>> +#define DEBUGFS_ALLOW_API	BIT(0)
>> +#define DEBUGFS_ALLOW_FS	BIT(1)
> #define DEBUGFS_ALLOW_API	BIT(0)
> #define DEBUGFS_ALLOW_MOUNT	BIT(1)
>
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS | DEBUGFS_ALLOW_API)
>> +#endif
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API)
>> +#endif
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
>> +#endif
>> +
>>  #endif /* _DEBUGFS_INTERNAL_H_ */
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 9ad9210d70a1..b609ad7c1343 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -476,6 +476,38 @@ config DEBUG_FS
>>  
>>  	  If unsure, say N.
>>  
>> +choice
>> +	prompt "Debugfs default access"
>> +	depends on DEBUG_FS
>> +	default DEBUG_FS_ACCESS_ALL
> default DEBUGFS_FS_ALLOW_ALL
>
>> +	help
>> +	  This select the default access restricions for debugfs.
>> +	  It can be overridden with kernel command line option
>> +	  debugfs=[on,no-fs,off] The restrictions apply for API access
>> +	  and filesystem registration. .
>> +
>> +config DEBUG_FS_ACCESS_ALL
> config DEBUG_FS_ALLOW_ALL
>
>> +       bool "Access normal"
>> +       help
>> +	  No restrictions applies. Both API and filesystem registration
>> +	  is on. This is the normal default operation.
>> +
>> +config DEBUG_FS_ACCESS_NO_FS
> config DEBUG_FS_DISALLOW_MOUNT
>
>> +       bool "Do not register debugfs as filesystem"
>> +       help
>> +	 The API is open but filesystem not loaded. Client can still do
>> +	 their work and readed with debug tools that does not need
>> +	 debugfs filesystem.
>> +
>> +config DEBUG_FS_ACCESS_NONE
> config DEBUG_FS_ALLOW_NONE
>
> Does that sound better?
>
> I'm not trying to be a pain, just trying to name this all correctly as
> it's messy with different config options and bits and mapping that to
> checks in the code without everything called the same.
>
> thanks,
>
> greg k-h

Re: [PATCH 2/2] debugfs: Add access restriction option

[Greg Kroah-Hartman]

On Wed, Jul 15, 2020 at 10:03:19AM +0000, Enderborg, Peter wrote:
> On 7/15/20 11:39 AM, Greg Kroah-Hartman wrote:
> > On Wed, Jul 15, 2020 at 10:42:07AM +0200, Peter Enderborg wrote:
> >> Since debugfs include sensitive information it need to be treated
> >> carefully. But it also has many very useful debug functions for userspace.
> >> With this option we can have same configuration for system with
> >> need of debugfs and a way to turn it off. This gives a extra protection
> >> for exposure on systems where user-space services with system
> >> access are attacked.
> >>
> >> It is controlled by a configurable default value that can be override
> >> with a kernel command line parameter. (debugfs=)
> >>
> >> It can be on or off, but also internally on but not seen from user-space.
> >> This no-fs mode do not register a debugfs as filesystem, but client can
> >> register their parts in the internal structures. This data can be readed
> >> with a debugger or saved with a crashkernel. When it is off clients
> >> get EPERM error when accessing the functions for registering their
> >> components.
> >>
> >> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> >> ---
> >>  .../admin-guide/kernel-parameters.txt         | 14 +++++++
> >>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
> >>  fs/debugfs/internal.h                         | 14 +++++++
> >>  lib/Kconfig.debug                             | 32 ++++++++++++++++
> >>  4 files changed, 97 insertions(+)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index fb95fad81c79..805aa2e58491 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -827,6 +827,20 @@
> >>  			useful to also enable the page_owner functionality.
> >>  			on: enable the feature
> >>  
> >> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
> >> +			and debugfs internal clients.
> >> +			Format: { on, no-fs, off }
> >> +			on: 	All functions are enabled.
> >> +			no-fs: 	Filesystem is not registered but kernel clients can
> >> +			        access APIs and a crashkernel can be used to read
> >> +				its content. There is nothing to mount.
> >> +			off: 	Filesystem is not registered and clients
> >> +			        get a -EPERM as result when trying to register files
> >> +				or directories within debugfs.
> >> +				This is equilivant of the runtime functionality if
> >> +				debugfs was not enabled in the kernel at all.
> >> +			Default value is set in build-time with a kernel configuration.
> > Naming is hard.  In looking at this, should "no-fs" be called
> > "no-mount"?  That's a better description of what it does, right?
> 
> I think no-fs cover it better since it does not register a filesystem
> but provides the interfaces. Mounting is then indirectly stopped.

But "mounting" is the common term we all know.  "no-fs" doesn't really
describe what is happening here, right?  Everything works internally
just fine, but we just are forbidding the filesystem to be mounted.

> The idea start with a check for mounting but it is much more
> definitely stopped by prevention of register of the filesystem.
> I can imagine to have a forth "mode" where it register a fs but
> not allowing mounting. Such mode maybe useful for some runtime
> configuration. But this patch is about boot time configuration.

Preventing the registering of the filesystem does cut out the ability to
mount the thing quite well :)

We could change it to just prevent the superblock from mounting if you
want, as maybe we do need the fs to remain in the list of filesystems in
the kernel at that point in time?  Otherwise we are lying to userspace.

thanks,

greg k-h

Re: [PATCH 2/2] debugfs: Add access restriction option

[Enderborg, Peter]

On 7/15/20 12:35 PM, Greg Kroah-Hartman wrote:
> On Wed, Jul 15, 2020 at 10:03:19AM +0000, Enderborg, Peter wrote:
>> On 7/15/20 11:39 AM, Greg Kroah-Hartman wrote:
>>> On Wed, Jul 15, 2020 at 10:42:07AM +0200, Peter Enderborg wrote:
>>>> Since debugfs include sensitive information it need to be treated
>>>> carefully. But it also has many very useful debug functions for userspace.
>>>> With this option we can have same configuration for system with
>>>> need of debugfs and a way to turn it off. This gives a extra protection
>>>> for exposure on systems where user-space services with system
>>>> access are attacked.
>>>>
>>>> It is controlled by a configurable default value that can be override
>>>> with a kernel command line parameter. (debugfs=)
>>>>
>>>> It can be on or off, but also internally on but not seen from user-space.
>>>> This no-fs mode do not register a debugfs as filesystem, but client can
>>>> register their parts in the internal structures. This data can be readed
>>>> with a debugger or saved with a crashkernel. When it is off clients
>>>> get EPERM error when accessing the functions for registering their
>>>> components.
>>>>
>>>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
>>>> ---
>>>>  .../admin-guide/kernel-parameters.txt         | 14 +++++++
>>>>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>>>>  fs/debugfs/internal.h                         | 14 +++++++
>>>>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>>>>  4 files changed, 97 insertions(+)
>>>>
>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>> index fb95fad81c79..805aa2e58491 100644
>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>> @@ -827,6 +827,20 @@
>>>>  			useful to also enable the page_owner functionality.
>>>>  			on: enable the feature
>>>>  
>>>> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
>>>> +			and debugfs internal clients.
>>>> +			Format: { on, no-fs, off }
>>>> +			on: 	All functions are enabled.
>>>> +			no-fs: 	Filesystem is not registered but kernel clients can
>>>> +			        access APIs and a crashkernel can be used to read
>>>> +				its content. There is nothing to mount.
>>>> +			off: 	Filesystem is not registered and clients
>>>> +			        get a -EPERM as result when trying to register files
>>>> +				or directories within debugfs.
>>>> +				This is equilivant of the runtime functionality if
>>>> +				debugfs was not enabled in the kernel at all.
>>>> +			Default value is set in build-time with a kernel configuration.
>>> Naming is hard.  In looking at this, should "no-fs" be called
>>> "no-mount"?  That's a better description of what it does, right?
>> I think no-fs cover it better since it does not register a filesystem
>> but provides the interfaces. Mounting is then indirectly stopped.
> But "mounting" is the common term we all know.  "no-fs" doesn't really
> describe what is happening here, right?  Everything works internally
> just fine, but we just are forbidding the filesystem to be mounted.
>
I have no objection but now you know the background. So no-mount then.

I will do a new patch-set.

>> The idea start with a check for mounting but it is much more
>> definitely stopped by prevention of register of the filesystem.
>> I can imagine to have a forth "mode" where it register a fs but
>> not allowing mounting. Such mode maybe useful for some runtime
>> configuration. But this patch is about boot time configuration.
> Preventing the registering of the filesystem does cut out the ability to
> mount the thing quite well :)
>
> We could change it to just prevent the superblock from mounting if you
> want, as maybe we do need the fs to remain in the list of filesystems in
> the kernel at that point in time?  Otherwise we are lying to userspace.

It is all about hide it away for userspace.


> thanks,
>
> greg k-h

Re: [PATCH 2/2] debugfs: Add access restriction option

[Enderborg, Peter]

On 7/15/20 11:39 AM, Greg Kroah-Hartman wrote:
> On Wed, Jul 15, 2020 at 10:42:07AM +0200, Peter Enderborg wrote:
>> Since debugfs include sensitive information it need to be treated
>> carefully. But it also has many very useful debug functions for userspace.
>> With this option we can have same configuration for system with
>> need of debugfs and a way to turn it off. This gives a extra protection
>> for exposure on systems where user-space services with system
>> access are attacked.
>>
>> It is controlled by a configurable default value that can be override
>> with a kernel command line parameter. (debugfs=)
>>
>> It can be on or off, but also internally on but not seen from user-space.
>> This no-fs mode do not register a debugfs as filesystem, but client can
>> register their parts in the internal structures. This data can be readed
>> with a debugger or saved with a crashkernel. When it is off clients
>> get EPERM error when accessing the functions for registering their
>> components.
>>
>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
>> ---
>>  .../admin-guide/kernel-parameters.txt         | 14 +++++++
>>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>>  fs/debugfs/internal.h                         | 14 +++++++
>>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>>  4 files changed, 97 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index fb95fad81c79..805aa2e58491 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -827,6 +827,20 @@
>>  			useful to also enable the page_owner functionality.
>>  			on: enable the feature
>>  
>> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
>> +			and debugfs internal clients.
>> +			Format: { on, no-fs, off }
>> +			on: 	All functions are enabled.
>> +			no-fs: 	Filesystem is not registered but kernel clients can
>> +			        access APIs and a crashkernel can be used to read
>> +				its content. There is nothing to mount.
>> +			off: 	Filesystem is not registered and clients
>> +			        get a -EPERM as result when trying to register files
>> +				or directories within debugfs.
>> +				This is equilivant of the runtime functionality if
>> +				debugfs was not enabled in the kernel at all.
>> +			Default value is set in build-time with a kernel configuration.
> Naming is hard.  In looking at this, should "no-fs" be called
> "no-mount"?  That's a better description of what it does, right?
One name that had in mind was Stealth. It is more of what it is intended to do than how.
> Then we can rename the bits to match the documentation so we aren't
> constantly thinking of different things and trying to match them up:
>
>
>> --- a/fs/debugfs/internal.h
>> +++ b/fs/debugfs/internal.h
>> @@ -29,4 +29,18 @@ struct debugfs_fsdata {
>>   */
>>  #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
>>  
>> +/* Access BITS */
>> +#define DEBUGFS_ALLOW_API	BIT(0)
>> +#define DEBUGFS_ALLOW_FS	BIT(1)
> #define DEBUGFS_ALLOW_API	BIT(0)
> #define DEBUGFS_ALLOW_MOUNT	BIT(1)
>
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_ALL
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_FS | DEBUGFS_ALLOW_API)
>> +#endif
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_NO_FS
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (DEBUGFS_ALLOW_API)
>> +#endif
>> +#ifdef CONFIG_DEBUG_FS_ACCESS_NONE
>> +#define DEFAULT_DEBUGFS_ACCESS_BITS (0)
>> +#endif
>> +
>>  #endif /* _DEBUGFS_INTERNAL_H_ */
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 9ad9210d70a1..b609ad7c1343 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -476,6 +476,38 @@ config DEBUG_FS
>>  
>>  	  If unsure, say N.
>>  
>> +choice
>> +	prompt "Debugfs default access"
>> +	depends on DEBUG_FS
>> +	default DEBUG_FS_ACCESS_ALL
> default DEBUGFS_FS_ALLOW_ALL
>
>> +	help
>> +	  This select the default access restricions for debugfs.
>> +	  It can be overridden with kernel command line option
>> +	  debugfs=[on,no-fs,off] The restrictions apply for API access
>> +	  and filesystem registration. .
>> +
>> +config DEBUG_FS_ACCESS_ALL
> config DEBUG_FS_ALLOW_ALL
>
>> +       bool "Access normal"
>> +       help
>> +	  No restrictions applies. Both API and filesystem registration
>> +	  is on. This is the normal default operation.
>> +
>> +config DEBUG_FS_ACCESS_NO_FS
> config DEBUG_FS_DISALLOW_MOUNT
>
>> +       bool "Do not register debugfs as filesystem"
>> +       help
>> +	 The API is open but filesystem not loaded. Client can still do
>> +	 their work and readed with debug tools that does not need
>> +	 debugfs filesystem.
>> +
>> +config DEBUG_FS_ACCESS_NONE
> config DEBUG_FS_ALLOW_NONE
>
> Does that sound better?
>
> I'm not trying to be a pain, just trying to name this all correctly as
> it's messy with different config options and bits and mapping that to
> checks in the code without everything called the same.
>
> thanks,
>
> greg k-h

[PATCH v6 0/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.
v4  Removed #ifdefs from inode.c and using internal.h for configuration
    and now using BIT() for that. Function is now always on, and are
    instead selected by a built in default or command line parameter.
    Changed return value on debug_mount
    Reported-by: kernel test robot <lkp@intel.com>
    Im not sure about that it is right
v5  Added notes to config help suggested by GregKH.
    Removed _BIT from names, white-space and tab.
    (checkpatch did not complain).
v6  Using ALLOW instead of ACCESS as name on BIT's. Change the fs to
    mount to make it clear and easy to understand. 

[PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Peter Enderborg]

This is a preparation for debugfs restricted mode.
We don't need debugfs to trace, the removed check stop tracefs to work
if debugfs is not initialised. We instead tries to automount within
debugfs and relay on it's handling. The code path is to create a
backward compatibility from when tracefs was part of debugfs, it is now
standalone and does not need debugfs. When debugfs is in restricted
it is compiled in but not active and return EPERM to clients and
tracefs wont work if it assumes it is active it is compiled in
kernel.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..848f67a5f16d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8945,9 +8945,7 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!tracefs_initialized()) ||
-		(IS_ENABLED(CONFIG_DEBUG_FS) &&
-		 WARN_ON(!debugfs_initialized())))
+	if (WARN_ON(!tracefs_initialized()))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.17.1

[PATCH 2/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

It is controlled by a configurable default value that can be override
with a kernel command line parameter. (debugfs=)

It can be on or off, but also internally on but not seen from user-space.
This no-mount mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 15 ++++++++
 fs/debugfs/inode.c                            | 37 +++++++++++++++++++
 fs/debugfs/internal.h                         | 14 +++++++
 lib/Kconfig.debug                             | 32 ++++++++++++++++
 4 files changed, 98 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..779d6cdc9627 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,21 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] This parameter enables what is exposed to userspace
+			and debugfs internal clients.
+			Format: { on, no-mount, off }
+			on: 	All functions are enabled.
+			no-mount:
+				Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				its content. There is nothing to mount.
+			off: 	Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+				This is equilivant of the runtime functionality if
+				debugfs was not enabled in the kernel at all.
+			Default value is set in build-time with a kernel configuration.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..02d08b17d0e6 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -35,6 +35,7 @@
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ALLOW_BITS;
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
 	struct dentry *dentry;
 	int error;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	pr_debug("creating file '%s'\n", name);
 
 	if (IS_ERR(parent))
@@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +808,25 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+	if (str && !strcmp(str, "on"))
+		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_MOUNT;
+	if (str && !strcmp(str, "no-mount"))
+		debugfs_allow = DEBUGFS_ALLOW_API;
+	if (str && !strcmp(str, "off"))
+		debugfs_allow = 0;
+
+	return 0;
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_MOUNT))
+		return -EPERM;
+
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 034e6973cead..92af8ae31313 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -29,4 +29,18 @@ struct debugfs_fsdata {
  */
 #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
 
+/* Access BITS */
+#define DEBUGFS_ALLOW_API	BIT(0)
+#define DEBUGFS_ALLOW_MOUNT	BIT(1)
+
+#ifdef CONFIG_DEBUG_FS_ALLOW_ALL
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_MOUNT | DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_DISALLOW_MOUNT
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_ALLOW_NONE
+#define DEFAULT_DEBUGFS_ALLOW_BITS (0)
+#endif
+
 #endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..aec81f38bfce 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -476,6 +476,38 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+choice
+	prompt "Debugfs default access"
+	depends on DEBUG_FS
+	default DEBUG_FS_ALLOW_ALL
+	help
+	  This select the default access restricions for debugfs.
+	  It can be overridden with kernel command line option
+	  debugfs=[on,no-mount,off] The restrictions apply for API access
+	  and filesystem registration. .
+
+config DEBUG_FS_ALLOW_ALL
+       bool "Access normal"
+       help
+	  No restrictions applies. Both API and filesystem registration
+	  is on. This is the normal default operation.
+
+config DEBUG_FS_DISALLOW_MOUNT
+       bool "Do not register debugfs as filesystem"
+       help
+	 The API is open but filesystem not loaded. Client can still do
+	 their work and readed with debug tools that does not need
+	 debugfs filesystem.
+
+config DEBUG_FS_ALLOW_NONE
+       bool "No access"
+       help
+	  Access is off. Clients get EPERM when trying to create nodes in
+	  debugfs tree and debugfs is not registred as an filesystem.
+	  Client can then back-off or continue without debugfs access.
+
+endchoice
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.17.1

Re: [PATCH 2/2] debugfs: Add access restriction option

[Randy Dunlap]

Hi,

On 7/15/20 8:25 AM, Peter Enderborg wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9ad9210d70a1..aec81f38bfce 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -476,6 +476,38 @@ config DEBUG_FS
>  
>  	  If unsure, say N.
>  
> +choice
> +	prompt "Debugfs default access"
> +	depends on DEBUG_FS
> +	default DEBUG_FS_ALLOW_ALL
> +	help
> +	  This select the default access restricions for debugfs.

	       selects                   restrictions
                 
> +	  It can be overridden with kernel command line option
> +	  debugfs=[on,no-mount,off] The restrictions apply for API access

	                      ,off]. The

> +	  and filesystem registration. .
> +
> +config DEBUG_FS_ALLOW_ALL
> +       bool "Access normal"
> +       help
> +	  No restrictions applies. Both API and filesystem registration

	                  apply.

> +	  is on. This is the normal default operation.
> +
> +config DEBUG_FS_DISALLOW_MOUNT
> +       bool "Do not register debugfs as filesystem"
> +       help
> +	 The API is open but filesystem not loaded. Client can still do
> +	 their work and readed with debug tools that does not need

	            and read                    that do not need

> +	 debugfs filesystem.
> +
> +config DEBUG_FS_ALLOW_NONE
> +       bool "No access"
> +       help
> +	  Access is off. Clients get EPERM when trying to create nodes in

	                             -EPERM

> +	  debugfs tree and debugfs is not registred as an filesystem.

	                                  registered as a filesystem.


> +	  Client can then back-off or continue without debugfs access.
> +
> +endchoice


Also, in many places in this Kconfig file, the indentation needs to be
fixed.  Some lines use spaces instead of one tab for indentation.
Help text (under "help") should be indented with one tab + 2 spaces.


-- 
~Randy

[PATCH v7 0/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.
v4  Removed #ifdefs from inode.c and using internal.h for configuration
    and now using BIT() for that. Function is now always on, and are
    instead selected by a built in default or command line parameter.
    Changed return value on debug_mount
    Reported-by: kernel test robot <lkp@intel.com>
    Im not sure about that it is right
v5  Added notes to config help suggested by GregKH.
    Removed _BIT from names, white-space and tab.
    (checkpatch did not complain).
v6  Using ALLOW instead of ACCESS as name on BIT's. Change the fs to
    mount to make it clear and easy to understand.
v7  Updated Kconfig.debug with Randy Dunlap corrections.

[PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Peter Enderborg]

This is a preparation for debugfs restricted mode.
We don't need debugfs to trace, the removed check stop tracefs to work
if debugfs is not initialised. We instead tries to automount within
debugfs and relay on it's handling. The code path is to create a
backward compatibility from when tracefs was part of debugfs, it is now
standalone and does not need debugfs. When debugfs is in restricted
it is compiled in but not active and return EPERM to clients and
tracefs wont work if it assumes it is active it is compiled in
kernel.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..848f67a5f16d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8945,9 +8945,7 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!tracefs_initialized()) ||
-		(IS_ENABLED(CONFIG_DEBUG_FS) &&
-		 WARN_ON(!debugfs_initialized())))
+	if (WARN_ON(!tracefs_initialized()))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.17.1

[PATCH 2/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

It is controlled by a configurable default value that can be override
with a kernel command line parameter. (debugfs=)

It can be on or off, but also internally on but not seen from user-space.
This no-mount mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 15 ++++++++
 fs/debugfs/inode.c                            | 37 +++++++++++++++++++
 fs/debugfs/internal.h                         | 14 +++++++
 lib/Kconfig.debug                             | 32 ++++++++++++++++
 4 files changed, 98 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..779d6cdc9627 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,21 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] This parameter enables what is exposed to userspace
+			and debugfs internal clients.
+			Format: { on, no-mount, off }
+			on: 	All functions are enabled.
+			no-mount:
+				Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				its content. There is nothing to mount.
+			off: 	Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+				This is equilivant of the runtime functionality if
+				debugfs was not enabled in the kernel at all.
+			Default value is set in build-time with a kernel configuration.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..02d08b17d0e6 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -35,6 +35,7 @@
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ALLOW_BITS;
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
 	struct dentry *dentry;
 	int error;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	pr_debug("creating file '%s'\n", name);
 
 	if (IS_ERR(parent))
@@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +808,25 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+	if (str && !strcmp(str, "on"))
+		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_MOUNT;
+	if (str && !strcmp(str, "no-mount"))
+		debugfs_allow = DEBUGFS_ALLOW_API;
+	if (str && !strcmp(str, "off"))
+		debugfs_allow = 0;
+
+	return 0;
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_MOUNT))
+		return -EPERM;
+
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 034e6973cead..92af8ae31313 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -29,4 +29,18 @@ struct debugfs_fsdata {
  */
 #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
 
+/* Access BITS */
+#define DEBUGFS_ALLOW_API	BIT(0)
+#define DEBUGFS_ALLOW_MOUNT	BIT(1)
+
+#ifdef CONFIG_DEBUG_FS_ALLOW_ALL
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_MOUNT | DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_DISALLOW_MOUNT
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_ALLOW_NONE
+#define DEFAULT_DEBUGFS_ALLOW_BITS (0)
+#endif
+
 #endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..ebe670fdf1bd 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -476,6 +476,38 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+choice
+	prompt "Debugfs default access"
+	depends on DEBUG_FS
+	default DEBUG_FS_ALLOW_ALL
+	help
+	  This selects the default access restrictions for debugfs.
+	  It can be overridden with kernel command line option
+	  debugfs=[on,no-mount,off]. The restrictions apply for API access
+	  and filesystem registration. .
+
+config DEBUG_FS_ALLOW_ALL
+	bool "Access normal"
+	help
+	  No restrictions apply. Both API and filesystem registration
+	  is on. This is the normal default operation.
+
+config DEBUG_FS_DISALLOW_MOUNT
+	bool "Do not register debugfs as filesystem"
+	help
+	  The API is open but filesystem not loaded. Client can still do
+	  their work and read with debug tools that do not need
+	  debugfs filesystem.
+
+config DEBUG_FS_ALLOW_NONE
+	bool "No access"
+	help
+	  Access is off. Clients get -PERM when trying to create nodes in
+	  debugfs tree and debugfs is not registered as a filesystem.
+	  Client can then back-off or continue without debugfs access.
+
+endchoice
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.17.1

Re: [PATCH 2/2] debugfs: Add access restriction option

[Randy Dunlap]

Hi Peter,

Here are a few more comments/corrections.

On 7/15/20 9:54 PM, Peter Enderborg wrote:
> Since debugfs include sensitive information it need to be treated
> carefully. But it also has many very useful debug functions for userspace.
> With this option we can have same configuration for system with
> need of debugfs and a way to turn it off. This gives a extra protection
> for exposure on systems where user-space services with system
> access are attacked.
> 
> It is controlled by a configurable default value that can be override
> with a kernel command line parameter. (debugfs=)
> 
> It can be on or off, but also internally on but not seen from user-space.
> This no-mount mode do not register a debugfs as filesystem, but client can
> register their parts in the internal structures. This data can be readed
> with a debugger or saved with a crashkernel. When it is off clients
> get EPERM error when accessing the functions for registering their
> components.
> 
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 15 ++++++++
>  fs/debugfs/inode.c                            | 37 +++++++++++++++++++
>  fs/debugfs/internal.h                         | 14 +++++++
>  lib/Kconfig.debug                             | 32 ++++++++++++++++
>  4 files changed, 98 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index fb95fad81c79..779d6cdc9627 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -827,6 +827,21 @@
>  			useful to also enable the page_owner functionality.
>  			on: enable the feature
>  
> +	debugfs=    	[KNL] This parameter enables what is exposed to userspace
> +			and debugfs internal clients.
> +			Format: { on, no-mount, off }
> +			on: 	All functions are enabled.
> +			no-mount:
> +				Filesystem is not registered but kernel clients can
> +			        access APIs and a crashkernel can be used to read
> +				its content. There is nothing to mount.
> +			off: 	Filesystem is not registered and clients
> +			        get a -EPERM as result when trying to register files
> +				or directories within debugfs.
> +				This is equilivant of the runtime functionality if

				        equivalent

> +				debugfs was not enabled in the kernel at all.
> +			Default value is set in build-time with a kernel configuration.
> +
>  	debugpat	[X86] Enable PAT debugging
>  
>  	decnet.addr=	[HW,NET]
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index b7f2e971ecbc..02d08b17d0e6 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c

> @@ -786,10 +808,25 @@ bool debugfs_initialized(void)
>  }
>  EXPORT_SYMBOL_GPL(debugfs_initialized);
>  

I would add some "else"s here:

> +static int __init debugfs_kernel(char *str)
> +{
> +	if (str && !strcmp(str, "on"))
> +		debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_MOUNT;
	else if ...

> +	if (str && !strcmp(str, "no-mount"))
> +		debugfs_allow = DEBUGFS_ALLOW_API;
	else if ...

> +	if (str && !strcmp(str, "off"))
> +		debugfs_allow = 0;
> +
> +	return 0;
> +}
> +early_param("debugfs", debugfs_kernel);

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9ad9210d70a1..ebe670fdf1bd 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -476,6 +476,38 @@ config DEBUG_FS
>  
>  	  If unsure, say N.
>  
> +choice
> +	prompt "Debugfs default access"
> +	depends on DEBUG_FS
> +	default DEBUG_FS_ALLOW_ALL
> +	help
> +	  This selects the default access restrictions for debugfs.
> +	  It can be overridden with kernel command line option
> +	  debugfs=[on,no-mount,off]. The restrictions apply for API access
> +	  and filesystem registration. .

stray '.' there.

> +
> +config DEBUG_FS_ALLOW_ALL
> +	bool "Access normal"
> +	help
> +	  No restrictions apply. Both API and filesystem registration
> +	  is on. This is the normal default operation.
> +
> +config DEBUG_FS_DISALLOW_MOUNT
> +	bool "Do not register debugfs as filesystem"
> +	help
> +	  The API is open but filesystem not loaded. Client can still do

	                  but filesystem is not loaded. Clients can still do

> +	  their work and read with debug tools that do not need
> +	  debugfs filesystem.
> +
> +config DEBUG_FS_ALLOW_NONE
> +	bool "No access"
> +	help
> +	  Access is off. Clients get -PERM when trying to create nodes in
> +	  debugfs tree and debugfs is not registered as a filesystem.
> +	  Client can then back-off or continue without debugfs access.
> +
> +endchoice
> +
>  source "lib/Kconfig.kgdb"
>  
>  source "lib/Kconfig.ubsan"
> 

thanks.
-- 
~Randy

[PATCH v8 0/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

v2. Removed MOUNT as part of restrictions. Added API's restrictions as
    separate restriction.
v3  Updated Documentation after Randy Dunlap reviews and suggestions.
v4  Removed #ifdefs from inode.c and using internal.h for configuration
    and now using BIT() for that. Function is now always on, and are
    instead selected by a built in default or command line parameter.
    Changed return value on debug_mount
    Reported-by: kernel test robot <lkp@intel.com>
    Im not sure about that it is right
v5  Added notes to config help suggested by GregKH.
    Removed _BIT from names, white-space and tab.
    (checkpatch did not complain).
v6  Using ALLOW instead of ACCESS as name on BIT's. Change the fs to
    mount to make it clear and easy to understand.
v7  Updated Kconfig.debug with Randy Dunlap corrections.
v8  Spell fixes from Randy and using else-if for command argument
    parser.
    

[PATCH 1/2] tracefs: Remove unnecessary debug_fs checks.

[Peter Enderborg]

This is a preparation for debugfs restricted mode.
We don't need debugfs to trace, the removed check stop tracefs to work
if debugfs is not initialised. We instead tries to automount within
debugfs and relay on it's handling. The code path is to create a
backward compatibility from when tracefs was part of debugfs, it is now
standalone and does not need debugfs. When debugfs is in restricted
it is compiled in but not active and return EPERM to clients and
tracefs wont work if it assumes it is active it is compiled in
kernel.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index bb62269724d5..848f67a5f16d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8945,9 +8945,7 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!tracefs_initialized()) ||
-		(IS_ENABLED(CONFIG_DEBUG_FS) &&
-		 WARN_ON(!debugfs_initialized())))
+	if (WARN_ON(!tracefs_initialized()))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.17.1

[PATCH 2/2] debugfs: Add access restriction option

[Peter Enderborg]

Since debugfs include sensitive information it need to be treated
carefully. But it also has many very useful debug functions for userspace.
With this option we can have same configuration for system with
need of debugfs and a way to turn it off. This gives a extra protection
for exposure on systems where user-space services with system
access are attacked.

It is controlled by a configurable default value that can be override
with a kernel command line parameter. (debugfs=)

It can be on or off, but also internally on but not seen from user-space.
This no-mount mode do not register a debugfs as filesystem, but client can
register their parts in the internal structures. This data can be readed
with a debugger or saved with a crashkernel. When it is off clients
get EPERM error when accessing the functions for registering their
components.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 .../admin-guide/kernel-parameters.txt         | 15 +++++++
 fs/debugfs/inode.c                            | 39 +++++++++++++++++++
 fs/debugfs/internal.h                         | 14 +++++++
 lib/Kconfig.debug                             | 32 +++++++++++++++
 4 files changed, 100 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index fb95fad81c79..6766a308ad96 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -827,6 +827,21 @@
 			useful to also enable the page_owner functionality.
 			on: enable the feature
 
+	debugfs=    	[KNL] This parameter enables what is exposed to userspace
+			and debugfs internal clients.
+			Format: { on, no-mount, off }
+			on: 	All functions are enabled.
+			no-mount:
+				Filesystem is not registered but kernel clients can
+			        access APIs and a crashkernel can be used to read
+				its content. There is nothing to mount.
+			off: 	Filesystem is not registered and clients
+			        get a -EPERM as result when trying to register files
+				or directories within debugfs.
+				This is equivalent of the runtime functionality if
+				debugfs was not enabled in the kernel at all.
+			Default value is set in build-time with a kernel configuration.
+
 	debugpat	[X86] Enable PAT debugging
 
 	decnet.addr=	[HW,NET]
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..2fcf66473436 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -35,6 +35,7 @@
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
+static unsigned int debugfs_allow = DEFAULT_DEBUGFS_ALLOW_BITS;
 
 /*
  * Don't allow access attributes to be changed whilst the kernel is locked down
@@ -266,6 +267,9 @@ static struct dentry *debug_mount(struct file_system_type *fs_type,
 			int flags, const char *dev_name,
 			void *data)
 {
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	return mount_single(fs_type, flags, data, debug_fill_super);
 }
 
@@ -311,6 +315,9 @@ static struct dentry *start_creating(const char *name, struct dentry *parent)
 	struct dentry *dentry;
 	int error;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API))
+		return ERR_PTR(-EPERM);
+
 	pr_debug("creating file '%s'\n", name);
 
 	if (IS_ERR(parent))
@@ -385,6 +392,11 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create file '%s'\n",
@@ -541,6 +553,11 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create directory '%s'\n",
@@ -583,6 +600,11 @@ struct dentry *debugfs_create_automount(const char *name,
 	if (IS_ERR(dentry))
 		return dentry;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_API)) {
+		failed_creating(dentry);
+		return ERR_PTR(-EPERM);
+	}
+
 	inode = debugfs_get_inode(dentry->d_sb);
 	if (unlikely(!inode)) {
 		pr_err("out of free dentries, can not create automount '%s'\n",
@@ -786,10 +808,27 @@ bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
+static int __init debugfs_kernel(char *str)
+{
+	if (str) {
+		if (!strcmp(str, "on"))
+			debugfs_allow = DEBUGFS_ALLOW_API | DEBUGFS_ALLOW_MOUNT;
+		else if (!strcmp(str, "no-mount"))
+			debugfs_allow = DEBUGFS_ALLOW_API;
+		else if (!strcmp(str, "off"))
+			debugfs_allow = 0;
+	}
+
+	return 0;
+}
+early_param("debugfs", debugfs_kernel);
 static int __init debugfs_init(void)
 {
 	int retval;
 
+	if (!(debugfs_allow & DEBUGFS_ALLOW_MOUNT))
+		return -EPERM;
+
 	retval = sysfs_create_mount_point(kernel_kobj, "debug");
 	if (retval)
 		return retval;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 034e6973cead..92af8ae31313 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -29,4 +29,18 @@ struct debugfs_fsdata {
  */
 #define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
 
+/* Access BITS */
+#define DEBUGFS_ALLOW_API	BIT(0)
+#define DEBUGFS_ALLOW_MOUNT	BIT(1)
+
+#ifdef CONFIG_DEBUG_FS_ALLOW_ALL
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_MOUNT | DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_DISALLOW_MOUNT
+#define DEFAULT_DEBUGFS_ALLOW_BITS (DEBUGFS_ALLOW_API)
+#endif
+#ifdef CONFIG_DEBUG_FS_ALLOW_NONE
+#define DEFAULT_DEBUGFS_ALLOW_BITS (0)
+#endif
+
 #endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..035bd3a63788 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -476,6 +476,38 @@ config DEBUG_FS
 
 	  If unsure, say N.
 
+choice
+	prompt "Debugfs default access"
+	depends on DEBUG_FS
+	default DEBUG_FS_ALLOW_ALL
+	help
+	  This selects the default access restrictions for debugfs.
+	  It can be overridden with kernel command line option
+	  debugfs=[on,no-mount,off]. The restrictions apply for API access
+	  and filesystem registration.
+
+config DEBUG_FS_ALLOW_ALL
+	bool "Access normal"
+	help
+	  No restrictions apply. Both API and filesystem registration
+	  is on. This is the normal default operation.
+
+config DEBUG_FS_DISALLOW_MOUNT
+	bool "Do not register debugfs as filesystem"
+	help
+	  The API is open but filesystem is not loaded. Clients can still do
+	  their work and read with debug tools that do not need
+	  debugfs filesystem.
+
+config DEBUG_FS_ALLOW_NONE
+	bool "No access"
+	help
+	  Access is off. Clients get -PERM when trying to create nodes in
+	  debugfs tree and debugfs is not registered as a filesystem.
+	  Client can then back-off or continue without debugfs access.
+
+endchoice
+
 source "lib/Kconfig.kgdb"
 
 source "lib/Kconfig.ubsan"
-- 
2.17.1

Re: [PATCH 2/2] debugfs: Add access restriction option

[Randy Dunlap]

On 7/16/20 12:15 AM, Peter Enderborg wrote:
> ---
>  .../admin-guide/kernel-parameters.txt         | 15 +++++++
>  fs/debugfs/inode.c                            | 39 +++++++++++++++++++
>  fs/debugfs/internal.h                         | 14 +++++++
>  lib/Kconfig.debug                             | 32 +++++++++++++++
>  4 files changed, 100 insertions(+)
> 

Hi Peter,
The changes look good. Thanks.

-- 
~Randy

Re: [PATCH v8 0/2] debugfs: Add access restriction option

[Greg Kroah-Hartman]

On Thu, Jul 16, 2020 at 09:15:09AM +0200, Peter Enderborg wrote:
> Since debugfs include sensitive information it need to be treated
> carefully. But it also has many very useful debug functions for userspace.
> With this option we can have same configuration for system with
> need of debugfs and a way to turn it off. This gives a extra protection
> for exposure on systems where user-space services with system
> access are attacked.
> 
> v2. Removed MOUNT as part of restrictions. Added API's restrictions as
>     separate restriction.
> v3  Updated Documentation after Randy Dunlap reviews and suggestions.
> v4  Removed #ifdefs from inode.c and using internal.h for configuration
>     and now using BIT() for that. Function is now always on, and are
>     instead selected by a built in default or command line parameter.
>     Changed return value on debug_mount
>     Reported-by: kernel test robot <lkp@intel.com>
>     Im not sure about that it is right
> v5  Added notes to config help suggested by GregKH.
>     Removed _BIT from names, white-space and tab.
>     (checkpatch did not complain).
> v6  Using ALLOW instead of ACCESS as name on BIT's. Change the fs to
>     mount to make it clear and easy to understand.
> v7  Updated Kconfig.debug with Randy Dunlap corrections.
> v8  Spell fixes from Randy and using else-if for command argument
>     parser.
>     
> 

Thanks for sticking with this, now queued up!

greg k-h