Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Pavel Machek]

On Thu 2018-04-19 15:35:47, David Howells wrote:
> Pavel Machek  wrote:
> 
> > >  (1) chmod and chown are disallowed on debugfs objects (though the root 
> > > dir
> > >  can be modified by mount and remount, but I'm not worried about 
> > > that).
> > 
> > This has nothing to do with the lockdown goals, right? I find chown of
> > such files quite nice, to allow debugging without doing sudo all the time.
> 
> It allows someone to give everyone access to files that should perhaps only be
> accessible by root.  Besides, if you disable lockdown then you can do this if
> you want.

As I said this has nothing to do with lockdown, so does not belong in
this series. (And besides, it is bad idea.)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Pavel Machek]

On Thu 2018-04-19 15:35:47, David Howells wrote:
> Pavel Machek  wrote:
> 
> > >  (1) chmod and chown are disallowed on debugfs objects (though the root 
> > > dir
> > >  can be modified by mount and remount, but I'm not worried about 
> > > that).
> > 
> > This has nothing to do with the lockdown goals, right? I find chown of
> > such files quite nice, to allow debugging without doing sudo all the time.
> 
> It allows someone to give everyone access to files that should perhaps only be
> accessible by root.  Besides, if you disable lockdown then you can do this if
> you want.

As I said this has nothing to do with lockdown, so does not belong in
this series. (And besides, it is bad idea.)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Pavel Machek  wrote:

> >  (1) chmod and chown are disallowed on debugfs objects (though the root dir
> >  can be modified by mount and remount, but I'm not worried about that).
> 
> This has nothing to do with the lockdown goals, right? I find chown of
> such files quite nice, to allow debugging without doing sudo all the time.

It allows someone to give everyone access to files that should perhaps only be
accessible by root.  Besides, if you disable lockdown then you can do this if
you want.

> >  (2) When the kernel is locked down, only files with the following criteria
> >  are permitted to be opened:
> > 
> > - The file must have mode 00444
> > - The file must not have ioctl methods
> > - The file must not have mmap
> 
> Dunno. Would not it be nicer to go through the debugfs files and split
> them into safe/unsafe varieties?

Whilst that is a laudable idea, there are at least a couple of thousand files
to analyse, some of them doing weird stuff in drivers that aren't easy to
understand, and some of them with lists of files, some of which might be safe
and some of which aren't safe.  Even some reads that look like they ought to
be safe have side effects (such as read-and-reset counters).

Besides, define 'safe'.  Is reading a particular reg on a device and returning
the value through a debugfs read safe, for example?  What about files where
reading is harmless, but writing needs to be disallowed?

I did try initially passing in a flag to say whether something was safe or
not, abusing an inode flag to store it since debugfs uses a plain inode
struct, but then I realised just how many ways there are to create debugfs
files and it started to get out of hand.  My initial idea also included
locking everything down by default and marking things unlocked on an as-needed
basis.

If you have the time to audit all these files, then that would be great.

I lean more towards the lock debugfs down entirely side.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Pavel Machek  wrote:

> >  (1) chmod and chown are disallowed on debugfs objects (though the root dir
> >  can be modified by mount and remount, but I'm not worried about that).
> 
> This has nothing to do with the lockdown goals, right? I find chown of
> such files quite nice, to allow debugging without doing sudo all the time.

It allows someone to give everyone access to files that should perhaps only be
accessible by root.  Besides, if you disable lockdown then you can do this if
you want.

> >  (2) When the kernel is locked down, only files with the following criteria
> >  are permitted to be opened:
> > 
> > - The file must have mode 00444
> > - The file must not have ioctl methods
> > - The file must not have mmap
> 
> Dunno. Would not it be nicer to go through the debugfs files and split
> them into safe/unsafe varieties?

Whilst that is a laudable idea, there are at least a couple of thousand files
to analyse, some of them doing weird stuff in drivers that aren't easy to
understand, and some of them with lists of files, some of which might be safe
and some of which aren't safe.  Even some reads that look like they ought to
be safe have side effects (such as read-and-reset counters).

Besides, define 'safe'.  Is reading a particular reg on a device and returning
the value through a debugfs read safe, for example?  What about files where
reading is harmless, but writing needs to be disallowed?

I did try initially passing in a flag to say whether something was safe or
not, abusing an inode flag to store it since debugfs uses a plain inode
struct, but then I realised just how many ways there are to create debugfs
files and it started to get out of hand.  My initial idea also included
locking everything down by default and marking things unlocked on an as-needed
basis.

If you have the time to audit all these files, then that would be great.

I lean more towards the lock debugfs down entirely side.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Pavel Machek]

On Wed 2018-04-11 17:27:16, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
> 
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).

This has nothing to do with the lockdown goals, right? I find chown of
such files quite nice, to allow debugging without doing sudo all the time.

>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
> 
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap

Dunno. Would not it be nicer to go through the debugfs files and split
them into safe/unsafe varieties?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Pavel Machek]

On Wed 2018-04-11 17:27:16, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
> 
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).

This has nothing to do with the lockdown goals, right? I find chown of
such files quite nice, to allow debugging without doing sudo all the time.

>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
> 
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap

Dunno. Would not it be nicer to go through the debugfs files and split
them into safe/unsafe varieties?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Andy Lutomirski]

On Thu, Apr 12, 2018 at 1:23 AM, Greg KH  wrote:
> On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
>> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> >> Greg KH  wrote:
>> >>
>> >> > Why not just disable debugfs entirely?  This half-hearted way to sorta
>> >> > lock it down is odd, it is meant to not be there at all, nothing in your
>> >> > normal system should ever depend on it.
>> >> >
>> >> > So again just don't allow it to be mounted at all, much simpler and more
>> >> > obvious as to what is going on.
>> >>
>> >> Yeah, I agree - and then I got complaints because it seems that it's been
>> >> abused to allow drivers and userspace components to communicate.
>> >
>> > With in-kernel code?  Please let me know and I'll go fix it up to not
>> > allow that, as that is not ok.
>> >
>> > I do know of some bad examples of out-of-tree code abusing debugfs to do
>> > crazy things (battery level monitoring?), but that's their own fault...
>> >
>> > debugfs is for DEBUGGING!  For anything you all feel should be "secure",
>> > then just disable it entirely.
>> >
>>
>> Debugfs is very, very useful for, ahem, debugging.  I really think
>> this is an example of why we should split lockdown into the read and
>> write varieties and allow mounting and reading debugfs when only write
>> is locked down.
>
> Ok, but be sure that there are no "secrets" in those debugging files if
> you really buy into the whole "lock down" mess...
>
> Really, it's easier to just disable the whole thing.
>

I mostly agree with your sentiment.  I'm saying that, for most uses, I
*don't* buy into the idea that a normal secure-boot-supporting distro
should block debugfs.  I sometimes like to ask people who report
problems to send me the contents of such-and-such file in debugfs, and
I think it should keep working.  Blocking write access to debugfs is
mostly sensible for a lockdown system, but blocking read only makes
sense if you're worried about straight-up bugs or if you think that
debugfs contains protection-worthy secrets.

What I want to see is:

lockdown=protect_integrity: debugfs is read-only, bpf and perf are
unrestricted, iopl and ioperm are disabled, etc.

lockdown=protect_integrity_and_secrecy: debugfs is gone, bpf and perf
are restricted, plus all the restrictions from
lockdown=protect_integrity

Distros should strongly prefer lockdown=protect_integrity (or
lockdown=off) by default.  lockdown=protect_integrity_and_secrecy is
for custom setups, embedded applications, etc.


--Andy

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Andy Lutomirski]

On Thu, Apr 12, 2018 at 1:23 AM, Greg KH  wrote:
> On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
>> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> >> Greg KH  wrote:
>> >>
>> >> > Why not just disable debugfs entirely?  This half-hearted way to sorta
>> >> > lock it down is odd, it is meant to not be there at all, nothing in your
>> >> > normal system should ever depend on it.
>> >> >
>> >> > So again just don't allow it to be mounted at all, much simpler and more
>> >> > obvious as to what is going on.
>> >>
>> >> Yeah, I agree - and then I got complaints because it seems that it's been
>> >> abused to allow drivers and userspace components to communicate.
>> >
>> > With in-kernel code?  Please let me know and I'll go fix it up to not
>> > allow that, as that is not ok.
>> >
>> > I do know of some bad examples of out-of-tree code abusing debugfs to do
>> > crazy things (battery level monitoring?), but that's their own fault...
>> >
>> > debugfs is for DEBUGGING!  For anything you all feel should be "secure",
>> > then just disable it entirely.
>> >
>>
>> Debugfs is very, very useful for, ahem, debugging.  I really think
>> this is an example of why we should split lockdown into the read and
>> write varieties and allow mounting and reading debugfs when only write
>> is locked down.
>
> Ok, but be sure that there are no "secrets" in those debugging files if
> you really buy into the whole "lock down" mess...
>
> Really, it's easier to just disable the whole thing.
>

I mostly agree with your sentiment.  I'm saying that, for most uses, I
*don't* buy into the idea that a normal secure-boot-supporting distro
should block debugfs.  I sometimes like to ask people who report
problems to send me the contents of such-and-such file in debugfs, and
I think it should keep working.  Blocking write access to debugfs is
mostly sensible for a lockdown system, but blocking read only makes
sense if you're worried about straight-up bugs or if you think that
debugfs contains protection-worthy secrets.

What I want to see is:

lockdown=protect_integrity: debugfs is read-only, bpf and perf are
unrestricted, iopl and ioperm are disabled, etc.

lockdown=protect_integrity_and_secrecy: debugfs is gone, bpf and perf
are restricted, plus all the restrictions from
lockdown=protect_integrity

Distros should strongly prefer lockdown=protect_integrity (or
lockdown=off) by default.  lockdown=protect_integrity_and_secrecy is
for custom setups, embedded applications, etc.


--Andy

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> >> Greg KH  wrote:
> >>
> >> > Why not just disable debugfs entirely?  This half-hearted way to sorta
> >> > lock it down is odd, it is meant to not be there at all, nothing in your
> >> > normal system should ever depend on it.
> >> >
> >> > So again just don't allow it to be mounted at all, much simpler and more
> >> > obvious as to what is going on.
> >>
> >> Yeah, I agree - and then I got complaints because it seems that it's been
> >> abused to allow drivers and userspace components to communicate.
> >
> > With in-kernel code?  Please let me know and I'll go fix it up to not
> > allow that, as that is not ok.
> >
> > I do know of some bad examples of out-of-tree code abusing debugfs to do
> > crazy things (battery level monitoring?), but that's their own fault...
> >
> > debugfs is for DEBUGGING!  For anything you all feel should be "secure",
> > then just disable it entirely.
> >
> 
> Debugfs is very, very useful for, ahem, debugging.  I really think
> this is an example of why we should split lockdown into the read and
> write varieties and allow mounting and reading debugfs when only write
> is locked down.

Ok, but be sure that there are no "secrets" in those debugging files if
you really buy into the whole "lock down" mess...

Really, it's easier to just disable the whole thing.

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
> > On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> >> Greg KH  wrote:
> >>
> >> > Why not just disable debugfs entirely?  This half-hearted way to sorta
> >> > lock it down is odd, it is meant to not be there at all, nothing in your
> >> > normal system should ever depend on it.
> >> >
> >> > So again just don't allow it to be mounted at all, much simpler and more
> >> > obvious as to what is going on.
> >>
> >> Yeah, I agree - and then I got complaints because it seems that it's been
> >> abused to allow drivers and userspace components to communicate.
> >
> > With in-kernel code?  Please let me know and I'll go fix it up to not
> > allow that, as that is not ok.
> >
> > I do know of some bad examples of out-of-tree code abusing debugfs to do
> > crazy things (battery level monitoring?), but that's their own fault...
> >
> > debugfs is for DEBUGGING!  For anything you all feel should be "secure",
> > then just disable it entirely.
> >
> 
> Debugfs is very, very useful for, ahem, debugging.  I really think
> this is an example of why we should split lockdown into the read and
> write varieties and allow mounting and reading debugfs when only write
> is locked down.

Ok, but be sure that there are no "secrets" in those debugging files if
you really buy into the whole "lock down" mess...

Really, it's easier to just disable the whole thing.

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Andy Lutomirski]

On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
> On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> Greg KH  wrote:
>>
>> > Why not just disable debugfs entirely?  This half-hearted way to sorta
>> > lock it down is odd, it is meant to not be there at all, nothing in your
>> > normal system should ever depend on it.
>> >
>> > So again just don't allow it to be mounted at all, much simpler and more
>> > obvious as to what is going on.
>>
>> Yeah, I agree - and then I got complaints because it seems that it's been
>> abused to allow drivers and userspace components to communicate.
>
> With in-kernel code?  Please let me know and I'll go fix it up to not
> allow that, as that is not ok.
>
> I do know of some bad examples of out-of-tree code abusing debugfs to do
> crazy things (battery level monitoring?), but that's their own fault...
>
> debugfs is for DEBUGGING!  For anything you all feel should be "secure",
> then just disable it entirely.
>

Debugfs is very, very useful for, ahem, debugging.  I really think
this is an example of why we should split lockdown into the read and
write varieties and allow mounting and reading debugfs when only write
is locked down.

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Andy Lutomirski]

On Wed, Apr 11, 2018 at 1:33 PM, Greg KH  wrote:
> On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
>> Greg KH  wrote:
>>
>> > Why not just disable debugfs entirely?  This half-hearted way to sorta
>> > lock it down is odd, it is meant to not be there at all, nothing in your
>> > normal system should ever depend on it.
>> >
>> > So again just don't allow it to be mounted at all, much simpler and more
>> > obvious as to what is going on.
>>
>> Yeah, I agree - and then I got complaints because it seems that it's been
>> abused to allow drivers and userspace components to communicate.
>
> With in-kernel code?  Please let me know and I'll go fix it up to not
> allow that, as that is not ok.
>
> I do know of some bad examples of out-of-tree code abusing debugfs to do
> crazy things (battery level monitoring?), but that's their own fault...
>
> debugfs is for DEBUGGING!  For anything you all feel should be "secure",
> then just disable it entirely.
>

Debugfs is very, very useful for, ahem, debugging.  I really think
this is an example of why we should split lockdown into the read and
write varieties and allow mounting and reading debugfs when only write
is locked down.

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> Greg KH  wrote:
> 
> > Why not just disable debugfs entirely?  This half-hearted way to sorta
> > lock it down is odd, it is meant to not be there at all, nothing in your
> > normal system should ever depend on it.
> > 
> > So again just don't allow it to be mounted at all, much simpler and more
> > obvious as to what is going on.
> 
> Yeah, I agree - and then I got complaints because it seems that it's been
> abused to allow drivers and userspace components to communicate.

With in-kernel code?  Please let me know and I'll go fix it up to not
allow that, as that is not ok.

I do know of some bad examples of out-of-tree code abusing debugfs to do
crazy things (battery level monitoring?), but that's their own fault...

debugfs is for DEBUGGING!  For anything you all feel should be "secure",
then just disable it entirely.

thanks,

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> Greg KH  wrote:
> 
> > Why not just disable debugfs entirely?  This half-hearted way to sorta
> > lock it down is odd, it is meant to not be there at all, nothing in your
> > normal system should ever depend on it.
> > 
> > So again just don't allow it to be mounted at all, much simpler and more
> > obvious as to what is going on.
> 
> Yeah, I agree - and then I got complaints because it seems that it's been
> abused to allow drivers and userspace components to communicate.

With in-kernel code?  Please let me know and I'll go fix it up to not
allow that, as that is not ok.

I do know of some bad examples of out-of-tree code abusing debugfs to do
crazy things (battery level monitoring?), but that's their own fault...

debugfs is for DEBUGGING!  For anything you all feel should be "secure",
then just disable it entirely.

thanks,

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Eric W. Biederman]

David Howells  writes:

> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
>
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).
>
>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
>
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap
>
>  (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs 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.
>
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
the code quality of debugfs was fine for debugging but debugfs was not
safe to mount on a production system.

Maybe the code quality is better now but for a filesystem that is
not supposed to be needed for developers letting us mount debugfs
seems odd.

Eric

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Eric W. Biederman]

David Howells  writes:

> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
>
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).
>
>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
>
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap
>
>  (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs 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.
>
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
the code quality of debugfs was fine for debugging but debugfs was not
safe to mount on a production system.

Maybe the code quality is better now but for a filesystem that is
not supposed to be needed for developers letting us mount debugfs
seems odd.

Eric

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Greg KH  wrote:

> Why not just disable debugfs entirely?  This half-hearted way to sorta
> lock it down is odd, it is meant to not be there at all, nothing in your
> normal system should ever depend on it.
> 
> So again just don't allow it to be mounted at all, much simpler and more
> obvious as to what is going on.

Yeah, I agree - and then I got complaints because it seems that it's been
abused to allow drivers and userspace components to communicate.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Greg KH  wrote:

> Why not just disable debugfs entirely?  This half-hearted way to sorta
> lock it down is odd, it is meant to not be there at all, nothing in your
> normal system should ever depend on it.
> 
> So again just don't allow it to be mounted at all, much simpler and more
> obvious as to what is going on.

Yeah, I agree - and then I got complaints because it seems that it's been
abused to allow drivers and userspace components to communicate.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Eric W. Biederman  wrote:

> Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
> the code quality of debugfs was fine for debugging but debugfs was not
> safe to mount on a production system.
> 
> Maybe the code quality is better now but for a filesystem that is
> not supposed to be needed for developers letting us mount debugfs
> seems odd.

I agree.  But debugfs has been abused and it seems that there are some things
that use it as an interface between a kernel driver and the userspace side.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[David Howells]

Eric W. Biederman  wrote:

> Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
> the code quality of debugfs was fine for debugging but debugfs was not
> safe to mount on a production system.
> 
> Maybe the code quality is better now but for a filesystem that is
> not supposed to be needed for developers letting us mount debugfs
> seems odd.

I agree.  But debugfs has been abused and it seems that there are some things
that use it as an interface between a kernel driver and the userspace side.

David

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 05:27:16PM +0100, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
> 
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).
> 
>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
> 
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap
> 
>  (3) When the kernel is locked down, files may only be opened for reading.
> 
> Normal device interaction should be done through configfs, sysfs 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.
> 
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why not just disable debugfs entirely?  This half-hearted way to sorta
lock it down is odd, it is meant to not be there at all, nothing in your
normal system should ever depend on it.

So again just don't allow it to be mounted at all, much simpler and more
obvious as to what is going on.

thanks,

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Greg KH]

On Wed, Apr 11, 2018 at 05:27:16PM +0100, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
> 
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>  can be modified by mount and remount, but I'm not worried about that).
> 
>  (2) When the kernel is locked down, only files with the following criteria
>  are permitted to be opened:
> 
>   - The file must have mode 00444
>   - The file must not have ioctl methods
>   - The file must not have mmap
> 
>  (3) When the kernel is locked down, files may only be opened for reading.
> 
> Normal device interaction should be done through configfs, sysfs 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.
> 
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why not just disable debugfs entirely?  This half-hearted way to sorta
lock it down is odd, it is meant to not be there at all, nothing in your
normal system should ever depend on it.

So again just don't allow it to be mounted at all, much simpler and more
obvious as to what is going on.

thanks,

greg k-h

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Randy Dunlap]

On 04/11/2018 09:27 AM, David Howells wrote:

> Signed-off-by: David Howells 
> cc: Andy Shevchenko 
> cc: acpi4asus-u...@lists.sourceforge.net
> cc: platform-driver-...@vger.kernel.org
> cc: Matthew Garrett 
> cc: Thomas Gleixner 
> ---
meta-comment:

I have been dinged for not spelling "cc:" as "Cc:". I really think that
either way should be acceptable.

-- 
~Randy

Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down

[Randy Dunlap]

On 04/11/2018 09:27 AM, David Howells wrote:

> Signed-off-by: David Howells 
> cc: Andy Shevchenko 
> cc: acpi4asus-u...@lists.sourceforge.net
> cc: platform-driver-...@vger.kernel.org
> cc: Matthew Garrett 
> cc: Thomas Gleixner 
> ---
meta-comment:

I have been dinged for not spelling "cc:" as "Cc:". I really think that
either way should be acceptable.

-- 
~Randy