[PATCH] debugfs: lockdown: Allow reading debugfs files that are not world readable

[PATCH] debugfs: lockdown: Allow reading debugfs files that are not world readable

[Michal Suchanek]

When the kernel is locked down the kernel allows reading only debugfs
files with mode 444. Mode 400 is also valid but is not allowed.

Make the 444 into a mask.

Fixes: 5496197f9b08 ("debugfs: Restrict debugfs when the kernel is locked down")
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 fs/debugfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 7d162b0efbf0..950c63fa4d0b 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -147,7 +147,7 @@ static int debugfs_locked_down(struct inode *inode,
 			       struct file *filp,
 			       const struct file_operations *real_fops)
 {
-	if ((inode->i_mode & 07777) == 0444 &&
+	if ((inode->i_mode & 07777 & ~0444) == 0 &&
 	    !(filp->f_mode & FMODE_WRITE) &&
 	    !real_fops->unlocked_ioctl &&
 	    !real_fops->compat_ioctl &&
-- 
2.31.1

Re: [PATCH] debugfs: lockdown: Allow reading debugfs files that are not world readable

[Greg Kroah-Hartman]

On Tue, Jan 04, 2022 at 06:05:05PM +0100, Michal Suchanek wrote:
> 
> When the kernel is locked down the kernel allows reading only debugfs
> files with mode 444. Mode 400 is also valid but is not allowed.
> 
> Make the 444 into a mask.
> 
> Fixes: 5496197f9b08 ("debugfs: Restrict debugfs when the kernel is locked down")
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  fs/debugfs/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Why has it taken so long for anyone to notice this (2 years!)?

Is that because no one uses the lockdown mode and tries to read debugfs
files?


> 
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 7d162b0efbf0..950c63fa4d0b 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -147,7 +147,7 @@ static int debugfs_locked_down(struct inode *inode,
>  			       struct file *filp,
>  			       const struct file_operations *real_fops)
>  {
> -	if ((inode->i_mode & 07777) == 0444 &&
> +	if ((inode->i_mode & 07777 & ~0444) == 0 &&

You are now allowing more than just 0400, is that intentional?

I never understood why files that were 0666 were not able to be read
here as well, why not allow that as well?  What was magic about 0444
files?

thanks,

greg k-h

Re: [PATCH] debugfs: lockdown: Allow reading debugfs files that are not world readable

[Michal Suchánek]

Hello,

On Wed, Jan 05, 2022 at 12:51:31PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 04, 2022 at 06:05:05PM +0100, Michal Suchanek wrote:
> > 
> > When the kernel is locked down the kernel allows reading only debugfs
> > files with mode 444. Mode 400 is also valid but is not allowed.
> > 
> > Make the 444 into a mask.
> > 
> > Fixes: 5496197f9b08 ("debugfs: Restrict debugfs when the kernel is locked down")
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  fs/debugfs/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Why has it taken so long for anyone to notice this (2 years!)?
> 
> Is that because no one uses the lockdown mode and tries to read debugfs
> files?

It's because people use those LTSS kernels that don't have this change.

> > 
> > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > index 7d162b0efbf0..950c63fa4d0b 100644
> > --- a/fs/debugfs/file.c
> > +++ b/fs/debugfs/file.c
> > @@ -147,7 +147,7 @@ static int debugfs_locked_down(struct inode *inode,
> >  			       struct file *filp,
> >  			       const struct file_operations *real_fops)
> >  {
> > -	if ((inode->i_mode & 07777) == 0444 &&
> > +	if ((inode->i_mode & 07777 & ~0444) == 0 &&
> 
> You are now allowing more than just 0400, is that intentional?

The intent is to allow files that have permissions that are subset of
0444. The only one that makes sense and people complain about is 0400
but if you had 0440 or 0004 it would be permitted as well.

> I never understood why files that were 0666 were not able to be read
> here as well, why not allow that as well?  What was magic about 0444
> files?

I don't understand that either but I am not really trying to challenge
that part.

Thanks

Michal

Re: [PATCH] debugfs: lockdown: Allow reading debugfs files that are not world readable

[Greg Kroah-Hartman]

On Wed, Jan 05, 2022 at 02:12:22PM +0100, Michal Suchánek wrote:
> Hello,
> 
> On Wed, Jan 05, 2022 at 12:51:31PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Jan 04, 2022 at 06:05:05PM +0100, Michal Suchanek wrote:
> > > 
> > > When the kernel is locked down the kernel allows reading only debugfs
> > > files with mode 444. Mode 400 is also valid but is not allowed.
> > > 
> > > Make the 444 into a mask.
> > > 
> > > Fixes: 5496197f9b08 ("debugfs: Restrict debugfs when the kernel is locked down")
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > ---
> > >  fs/debugfs/file.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Why has it taken so long for anyone to notice this (2 years!)?
> > 
> > Is that because no one uses the lockdown mode and tries to read debugfs
> > files?
> 
> It's because people use those LTSS kernels that don't have this change.
> 
> > > 
> > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > > index 7d162b0efbf0..950c63fa4d0b 100644
> > > --- a/fs/debugfs/file.c
> > > +++ b/fs/debugfs/file.c
> > > @@ -147,7 +147,7 @@ static int debugfs_locked_down(struct inode *inode,
> > >  			       struct file *filp,
> > >  			       const struct file_operations *real_fops)
> > >  {
> > > -	if ((inode->i_mode & 07777) == 0444 &&
> > > +	if ((inode->i_mode & 07777 & ~0444) == 0 &&
> > 
> > You are now allowing more than just 0400, is that intentional?
> 
> The intent is to allow files that have permissions that are subset of
> 0444. The only one that makes sense and people complain about is 0400
> but if you had 0440 or 0004 it would be permitted as well.
> 
> > I never understood why files that were 0666 were not able to be read
> > here as well, why not allow that as well?  What was magic about 0444
> > files?
> 
> I don't understand that either but I am not really trying to challenge
> that part.

Fair enough.  I don't care about the locked-down mode stuff, so I'll go
queue this up now, thanks!

greg k-h