Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Tue, 2017-04-18 at 16:30 +0100, David Howells wrote: > Ben Hutchings wrote: > > > So it's generally not going to be OK to turn off debugfs. There will > > probably need to be a distinction between believed-safe and unsafe > > directories/files. > > Any suggestion on how to mark this distinction? I don't know. > I'd prefer not to modify every read/write op associated with a > debugfs file. I think debugfs should be assumed unsafe by default. So only the believed-safe parts would need to be changed. > Modify > DEFINE_DEBUGFS_ATTRIBUTE() maybe? And provide lockable variants of > debugfs_create_u8() and co.? That could help. Ben. -- Ben Hutchings The world is coming to an end. Please log off. signature.asc Description: This is a digitally signed message part
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
Ben Hutchings wrote: > > Shouldn't this now appear under /sys/kernel/tracing/ ? > > True, but old tracing scripts didn't go away. Conversion to a symlink would fix that. David
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
Ben Hutchings wrote: > So it's generally not going to be OK to turn off debugfs. There will > probably need to be a distinction between believed-safe and unsafe > directories/files. Any suggestion on how to mark this distinction? I'd prefer not to modify every read/write op associated with a debugfs file. Modify DEFINE_DEBUGFS_ATTRIBUTE() maybe? And provide lockable variants of debugfs_create_u8() and co.? David
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Tue, 2017-04-18 at 15:55 +0100, David Howells wrote: > Ben Hutchings wrote: > > > - tracing (now tracefs, but it's expected to appear under debugfs) > > Shouldn't this now appear under /sys/kernel/tracing/ ? True, but old tracing scripts didn't go away. Ben. -- Ben Hutchings The world is coming to an end. Please log off. signature.asc Description: This is a digitally signed message part
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
Ben Hutchings wrote: > - tracing (now tracefs, but it's expected to appear under debugfs) Shouldn't this now appear under /sys/kernel/tracing/ ? David
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Tue, 2017-04-18 at 09:06 +0300, Andy Shevchenko wrote: > > On Mon, Apr 10, 2017 at 4:16 PM, David Howells wrote: > > > > Andy Shevchenko wrote: > > > > > > > It looks a bit fragile when responsility of whatever reasons kernel > > > > > can't serve become a driver burden. > > > > > Can we fix this in debugfs framework instead? > > > > > > > > Fix it with debugfs how? We can't offload the decision to userspace. > > > > > > I mean to do at least similar like you have done for module > > > parameters. So, instead of putting above code to each attribute in > > > question make a special (marked) attribute instead and debugfs > > > framework will know how to deal with that. > > > > Hmmm... It's tricky in that debugfs doesn't have any of its own structures, > > but is entirely built on standard VFS ones, so finding somewhere to store > > the > > information is going to be awkward. > > I see. > > > One obvious solution is to entirely lock > > down debugfs in secure boot more, but that might be a bit drastic. > > But this sounds sane! debugFS for debugging, not for production. If > someone is using secure kernel it means pure production use (otherwise > one may do temporary hacks in kernel). [...] Production systems need instrumentation to understand performance issues and any bugs that for whatever reason didn't show up in earlier testing. A number of interfaces for that have been added under debugfs: - tracing (now tracefs, but it's expected to appear under debugfs) - dynamic_debug - various ad-hoc statistics So it's generally not going to be OK to turn off debugfs. There will probably need to be a distinction between believed-safe and unsafe directories/files. Ben. -- Ben Hutchings The world is coming to an end. Please log off. signature.asc Description: This is a digitally signed message part
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Mon, Apr 10, 2017 at 4:16 PM, David Howells wrote: > Andy Shevchenko wrote: > >> >> It looks a bit fragile when responsility of whatever reasons kernel >> >> can't serve become a driver burden. >> >> Can we fix this in debugfs framework instead? >> > >> > Fix it with debugfs how? We can't offload the decision to userspace. >> >> I mean to do at least similar like you have done for module >> parameters. So, instead of putting above code to each attribute in >> question make a special (marked) attribute instead and debugfs >> framework will know how to deal with that. > > Hmmm... It's tricky in that debugfs doesn't have any of its own structures, > but is entirely built on standard VFS ones, so finding somewhere to store the > information is going to be awkward. I see. > One obvious solution is to entirely lock > down debugfs in secure boot more, but that might be a bit drastic. But this sounds sane! debugFS for debugging, not for production. If someone is using secure kernel it means pure production use (otherwise one may do temporary hacks in kernel). If one still needs debugfs in secure mode, it sounds to me as architectural bug in code in question. > > Note that it's still going to be a driver burden to some extent anyway. The > driver has to tell the core what needs to be restricted. > > Further, I guess configfs needs attention also. -- With Best Regards, Andy Shevchenko
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
Andy Shevchenko wrote: > >> It looks a bit fragile when responsility of whatever reasons kernel > >> can't serve become a driver burden. > >> Can we fix this in debugfs framework instead? > > > > Fix it with debugfs how? We can't offload the decision to userspace. > > I mean to do at least similar like you have done for module > parameters. So, instead of putting above code to each attribute in > question make a special (marked) attribute instead and debugfs > framework will know how to deal with that. Hmmm... It's tricky in that debugfs doesn't have any of its own structures, but is entirely built on standard VFS ones, so finding somewhere to store the information is going to be awkward. One obvious solution is to entirely lock down debugfs in secure boot more, but that might be a bit drastic. Note that it's still going to be a driver burden to some extent anyway. The driver has to tell the core what needs to be restricted. Further, I guess configfs needs attention also. David
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Fri, Apr 7, 2017 at 3:50 PM, David Howells wrote: > Andy Shevchenko wrote: > >> > From: Matthew Garrett >> > >> > We have no way of validating what all of the Asus WMI methods do on a given >> > machine - and there's a risk that some will allow hardware state to be >> > manipulated in such a way that arbitrary code can be executed in the >> > kernel, circumventing module loading restrictions. Prevent that if the >> > kernel is locked down. >> >> > + if (kernel_is_locked_down()) >> > + return -EPERM; >> >> It looks a bit fragile when responsility of whatever reasons kernel >> can't serve become a driver burden. >> Can we fix this in debugfs framework instead? > > Fix it with debugfs how? We can't offload the decision to userspace. I mean to do at least similar like you have done for module parameters. So, instead of putting above code to each attribute in question make a special (marked) attribute instead and debugfs framework will know how to deal with that. -- With Best Regards, Andy Shevchenko
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
Andy Shevchenko wrote: > > From: Matthew Garrett > > > > We have no way of validating what all of the Asus WMI methods do on a given > > machine - and there's a risk that some will allow hardware state to be > > manipulated in such a way that arbitrary code can be executed in the > > kernel, circumventing module loading restrictions. Prevent that if the > > kernel is locked down. > > > + if (kernel_is_locked_down()) > > + return -EPERM; > > It looks a bit fragile when responsility of whatever reasons kernel > can't serve become a driver burden. > Can we fix this in debugfs framework instead? Fix it with debugfs how? We can't offload the decision to userspace. David
Re: [PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
On Wed, Apr 5, 2017 at 11:16 PM, David Howells wrote: > From: Matthew Garrett > > We have no way of validating what all of the Asus WMI methods do on a given > machine - and there's a risk that some will allow hardware state to be > manipulated in such a way that arbitrary code can be executed in the > kernel, circumventing module loading restrictions. Prevent that if the > kernel is locked down. > + if (kernel_is_locked_down()) > + return -EPERM; It looks a bit fragile when responsility of whatever reasons kernel can't serve become a driver burden. Can we fix this in debugfs framework instead? > + > err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval); > > if (err < 0) > @@ -1916,6 +1919,9 @@ static int show_devs(struct seq_file *m, void *data) > int err; > u32 retval = -1; > > + if (kernel_is_locked_down()) > + return -EPERM; > + > err = asus_wmi_set_devstate(asus->debug.dev_id, > asus->debug.ctrl_param, > &retval); > > @@ -1940,6 +1946,9 @@ static int show_call(struct seq_file *m, void *data) > union acpi_object *obj; > acpi_status status; > > + if (kernel_is_locked_down()) > + return -EPERM; > + > status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, > 1, asus->debug.method_id, > &input, &output); > -- With Best Regards, Andy Shevchenko
[PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
From: Matthew Garrett We have no way of validating what all of the Asus WMI methods do on a given machine - and there's a risk that some will allow hardware state to be manipulated in such a way that arbitrary code can be executed in the kernel, circumventing module loading restrictions. Prevent that if the kernel is locked down. Signed-off-by: Matthew Garrett Signed-off-by: David Howells cc: acpi4asus-u...@lists.sourceforge.net cc: platform-driver-...@vger.kernel.org --- drivers/platform/x86/asus-wmi.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 8fe5890bf539..feef25076813 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -1900,6 +1900,9 @@ static int show_dsts(struct seq_file *m, void *data) int err; u32 retval = -1; + if (kernel_is_locked_down()) + return -EPERM; + err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval); if (err < 0) @@ -1916,6 +1919,9 @@ static int show_devs(struct seq_file *m, void *data) int err; u32 retval = -1; + if (kernel_is_locked_down()) + return -EPERM; + err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param, &retval); @@ -1940,6 +1946,9 @@ static int show_call(struct seq_file *m, void *data) union acpi_object *obj; acpi_status status; + if (kernel_is_locked_down()) + return -EPERM; + status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 1, asus->debug.method_id, &input, &output);
[PATCH 15/24] asus-wmi: Restrict debugfs interface when the kernel is locked down
From: Matthew Garrett We have no way of validating what all of the Asus WMI methods do on a given machine - and there's a risk that some will allow hardware state to be manipulated in such a way that arbitrary code can be executed in the kernel, circumventing module loading restrictions. Prevent that if the kernel is locked down. Signed-off-by: Matthew Garrett Signed-off-by: David Howells --- drivers/platform/x86/asus-wmi.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 8fe5890bf539..feef25076813 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -1900,6 +1900,9 @@ static int show_dsts(struct seq_file *m, void *data) int err; u32 retval = -1; + if (kernel_is_locked_down()) + return -EPERM; + err = asus_wmi_get_devstate(asus, asus->debug.dev_id, &retval); if (err < 0) @@ -1916,6 +1919,9 @@ static int show_devs(struct seq_file *m, void *data) int err; u32 retval = -1; + if (kernel_is_locked_down()) + return -EPERM; + err = asus_wmi_set_devstate(asus->debug.dev_id, asus->debug.ctrl_param, &retval); @@ -1940,6 +1946,9 @@ static int show_call(struct seq_file *m, void *data) union acpi_object *obj; acpi_status status; + if (kernel_is_locked_down()) + return -EPERM; + status = wmi_evaluate_method(ASUS_WMI_MGMT_GUID, 1, asus->debug.method_id, &input, &output);
