On 5/28/20 10:27 AM, Greg Kroah-Hartman wrote: > On Thu, May 28, 2020 at 10:00:31AM +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. It is needed new >> kernel command line parameter to be activated. > By "configuration" do you mean "kernel configuration"? What is wrong > with relying on the build option like we do today? > > You might want to reword all of this to make more sense about the > "problem" you are trying to solve here, as I don't really understand it, > sorry. > > >> Signed-off-by: Peter Enderborg <peter.enderb...@sony.com> >> --- >> fs/debugfs/inode.c | 17 ++++++++++++++++- >> lib/Kconfig.debug | 10 ++++++++++ >> 2 files changed, 26 insertions(+), 1 deletion(-) > No documentation update? That's not good :( > > >> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c >> index b7f2e971ecbc..bde37dab77e0 100644 >> --- a/fs/debugfs/inode.c >> +++ b/fs/debugfs/inode.c >> @@ -786,10 +786,25 @@ bool debugfs_initialized(void) >> } >> EXPORT_SYMBOL_GPL(debugfs_initialized); >> >> +static int allow_debugfs; >> + >> +static int __init debugfs_kernel(char *str) >> +{ >> + if (str && !strcmp(str, "true")) >> + allow_debugfs = true; >> + >> + return 0; >> + >> +} >> +early_param("debugfs", debugfs_kernel); >> + >> static int __init debugfs_init(void) >> { >> int retval; >> - >> +#ifdef CONFIG_DEBUG_FS_MOUNT_RESTRICTED >> + if (!allow_debugfs) >> + return -EPERM; >> +#endif > But you are not restricting the ability to mount it here, you are > removing the ability for it to even start up at all. What does this > break for code that thinks the filesystem is registered (i.e. the call > to simple_pin_fs() in start_creating() in fs/debugfs/inode.c? > If it does, the lines below is also cause the same problem.
> >> retval = sysfs_create_mount_point(kernel_kobj, "debug"); >> if (retval) >> return retval; >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug >> index 21d9c5f6e7ec..d3a3338740d2 100644 >> --- a/lib/Kconfig.debug >> +++ b/lib/Kconfig.debug >> @@ -443,6 +443,16 @@ config DEBUG_FS >> >> If unsure, say N. >> >> +config DEBUG_FS_MOUNT_RESTRICTED >> + bool "Debug Filesystem mount 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. > No hint as to what the command line option is? And again, it's not a > restriction for mounting it, you are preventing it from initializing > entirely. > > Which, as I have found out over time, can had side affects for codepaths > that try to check the return value of debugfs calls, and if they fail to > succeed, abort the whole driver/subsystem/whatever initializaion path, > which is not what I think you want to do here at all. > > thanks, > > greg k-h