| Message ID | 20200619165010.645925-58-seth.forshee@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Lockdown updates | expand |
On Fri, Jun 19, 2020 at 11:50:10AM -0500, Seth Forshee wrote: > From: David Howells <dhowells@redhat.com> > > BugLink: https://bugs.launchpad.net/bugs/1884159 > > Disallow opening of debugfs files when the kernel is locked down as various > drivers give raw access to hardware through debugfs. I was noticing that starting from disco the policy for debugfs is to permit access only to world-readable files when the kernel is locked down, but in bionic (and also here in xenial) we are preventing any kind of access to debugfs. I guess this is fine, since we want to stick with the upstream lockdown policy, but I just want to make sure this is what we want. -Andrea > > Accesses to tracefs should use /sys/kernel/tracing/ rather than > /sys/kernel/debug/tracing/. Possibly a symlink should be emplaced. > > Normal device interaction should be done through configfs or a miscdev, not > debugfs. > > Note that this makes it unnecessary to specifically lock down show_dsts(), > show_devs() and show_call() in the asus-wmi driver. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Andy Shevchenko <andy.shevchenko@gmail.com> > cc: acpi4asus-user@lists.sourceforge.net > cc: platform-driver-x86@vger.kernel.org > cc: Matthew Garrett <matthew.garrett@nebula.com> > cc: Thomas Gleixner <tglx@linutronix.de> > (backported from commit 125da2e1c5d0a6aca5faafba336c8e8506a4e000 > git://git.kernel.org/pub/scm/linux/kernel/git/jwboyer/fedora.git) > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > fs/debugfs/file.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > index 592059f88e04..98521a791504 100644 > --- a/fs/debugfs/file.c > +++ b/fs/debugfs/file.c > @@ -106,6 +106,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp) > const struct file_operations *real_fops = NULL; > int srcu_idx, r; > > + if (secure_modules()) > + return -EPERM; > + > r = debugfs_use_file_start(dentry, &srcu_idx); > if (r) { > r = -ENOENT; > @@ -235,6 +238,9 @@ static int full_proxy_open(struct inode *inode, struct file *filp) > struct file_operations *proxy_fops = NULL; > int srcu_idx, r; > > + if (secure_modules()) > + return -EPERM; > + > r = debugfs_use_file_start(dentry, &srcu_idx); > if (r) { > r = -ENOENT; > -- > 2.27.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Mon, Jun 22, 2020 at 10:45:53AM +0200, Andrea Righi wrote: > On Fri, Jun 19, 2020 at 11:50:10AM -0500, Seth Forshee wrote: > > From: David Howells <dhowells@redhat.com> > > > > BugLink: https://bugs.launchpad.net/bugs/1884159 > > > > Disallow opening of debugfs files when the kernel is locked down as various > > drivers give raw access to hardware through debugfs. > > I was noticing that starting from disco the policy for debugfs is to > permit access only to world-readable files when the kernel is locked > down, but in bionic (and also here in xenial) we are preventing any kind > of access to debugfs. > > I guess this is fine, since we want to stick with the upstream > lockdown policy, but I just want to make sure this is what we want. My strategy for xenial was to match the state of things in bionic, and this is a backport of the patch we have there. If we want to consider loosening this debugfs restriction for all the kernels prior to focal (which iirc is the first to have the world-readable exception) we can consider that separately. Seth > > -Andrea > > > > > Accesses to tracefs should use /sys/kernel/tracing/ rather than > > /sys/kernel/debug/tracing/. Possibly a symlink should be emplaced. > > > > Normal device interaction should be done through configfs or a miscdev, not > > debugfs. > > > > Note that this makes it unnecessary to specifically lock down show_dsts(), > > show_devs() and show_call() in the asus-wmi driver. > > > > Signed-off-by: David Howells <dhowells@redhat.com> > > cc: Andy Shevchenko <andy.shevchenko@gmail.com> > > cc: acpi4asus-user@lists.sourceforge.net > > cc: platform-driver-x86@vger.kernel.org > > cc: Matthew Garrett <matthew.garrett@nebula.com> > > cc: Thomas Gleixner <tglx@linutronix.de> > > (backported from commit 125da2e1c5d0a6aca5faafba336c8e8506a4e000 > > git://git.kernel.org/pub/scm/linux/kernel/git/jwboyer/fedora.git) > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > --- > > fs/debugfs/file.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c > > index 592059f88e04..98521a791504 100644 > > --- a/fs/debugfs/file.c > > +++ b/fs/debugfs/file.c > > @@ -106,6 +106,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp) > > const struct file_operations *real_fops = NULL; > > int srcu_idx, r; > > > > + if (secure_modules()) > > + return -EPERM; > > + > > r = debugfs_use_file_start(dentry, &srcu_idx); > > if (r) { > > r = -ENOENT; > > @@ -235,6 +238,9 @@ static int full_proxy_open(struct inode *inode, struct file *filp) > > struct file_operations *proxy_fops = NULL; > > int srcu_idx, r; > > > > + if (secure_modules()) > > + return -EPERM; > > + > > r = debugfs_use_file_start(dentry, &srcu_idx); > > if (r) { > > r = -ENOENT; > > -- > > 2.27.0 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 592059f88e04..98521a791504 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -106,6 +106,9 @@ static int open_proxy_open(struct inode *inode, struct file *filp) const struct file_operations *real_fops = NULL; int srcu_idx, r; + if (secure_modules()) + return -EPERM; + r = debugfs_use_file_start(dentry, &srcu_idx); if (r) { r = -ENOENT; @@ -235,6 +238,9 @@ static int full_proxy_open(struct inode *inode, struct file *filp) struct file_operations *proxy_fops = NULL; int srcu_idx, r; + if (secure_modules()) + return -EPERM; + r = debugfs_use_file_start(dentry, &srcu_idx); if (r) { r = -ENOENT;