Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1736670 > unrolled thread

[PATCH 00/12] Introduce support for Dell SMBIOS over WMI

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 08/12] platform/x86: wmi: Cleanup exit routine in reverse order of init Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor GUID modalias Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 02/12] platform/x86: dell-wmi: Don't match on descriptor  GUID modalias Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
    [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI interface Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:30 +0200
        RE: [PATCH 04/12] platform/x86: dell-smbios: Switch to a WMI-ACPI  interface <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200
    [PATCH 12/12] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 01/12] platform/x86: dell-wmi: label driver as handling notifications Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
        RE: [PATCH 01/12] platform/x86: dell-wmi: label driver as handling  notifications <Mario.Limonciello@dell.com> - 2017-09-25 22:20 +0200
    [PATCH 10/12] platform/x86: wmi: destroy on cleanup rather than unregister Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
    [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
        RE: [PATCH 07/12] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check <Mario.Limonciello@dell.com> - 2017-09-21 23:00 +0200
    [PATCH 09/12] platform/x86: wmi: create character devices when requested by drivers Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:00 +0200
      Re: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:50 +0200
        RE: [PATCH 09/12] platform/x86: wmi: create character devices when  requested by drivers <Mario.Limonciello@dell.com> - 2017-09-21 21:30 +0200
    [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
      Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-21 18:30 +0200
      Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition  to driver Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:10 +0200
    [PATCH 05/12] platform/x86: dell-smbios: rename to dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-21 16:10 +0200
    Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:20 +0200
      RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 18:40 +0200
        Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI Pali Rohár <pali.rohar@gmail.com> - 2017-09-25 18:50 +0200
          RE: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-25 21:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1739140 — Re: [PATCH 03/12] platform/x86: dell-smbios: Add pr_fmt definition to driver

On Thursday 21 September 2017 08:57:08 Mario Limonciello wrote:
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>  drivers/platform/x86/dell-smbios.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index 0a5723468bff..e9b1ca07c872 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -12,6 +12,7 @@
>   *  it under the terms of the GNU General Public License version 2 as
>   *  published by the Free Software Foundation.
>   */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>

After fixing Andy's comment, you can add my

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1736694 — [PATCH 05/12] platform/x86: dell-smbios: rename to dell-wmi-smbios

This follows the style of the rest of the platform x86 WMI drivers.

Renaming the driver requires adjusting the other drivers using
dell-smbios to pick up the newly named includes.

While renaming, I noticed that this driver was missing from
MAINTAINERs. Add it to that and myself to the list of people maintaing
it.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 MAINTAINERS                                          |  6 ++++++
 drivers/platform/x86/Kconfig                         |  2 +-
 drivers/platform/x86/Makefile                        |  2 +-
 drivers/platform/x86/dell-laptop.c                   |  2 +-
 .../x86/{dell-smbios.c => dell-wmi-smbios.c}         | 20 ++++++++++----------
 .../x86/{dell-smbios.h => dell-wmi-smbios.h}         |  4 ++--
 drivers/platform/x86/dell-wmi.c                      |  2 +-
 7 files changed, 22 insertions(+), 16 deletions(-)
 rename drivers/platform/x86/{dell-smbios.c => dell-wmi-smbios.c} (92%)
 rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (96%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5d8ea24a8ee7..437daa9062e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4002,6 +4002,12 @@ M:	Pali Rohár <pali.rohar@gmail.com>
 S:	Maintained
 F:	drivers/platform/x86/dell-wmi.c
 
+DELL WMI SMBIOS DRIVER
+M:	Pali Rohár <pali.rohar@gmail.com>
+M:	Mario Limonciello <mario.limonciello@dell.com>
+S:	Maintained
+F:	drivers/platform/x86/dell-wmi-smbios.c
+
 DELTA ST MEDIA DRIVER
 M:	Hugues Fruchet <hugues.fruchet@st.com>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 81d61c0f4ef8..a70bcd8caa72 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -91,7 +91,7 @@ config ASUS_LAPTOP
 
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
-config DELL_SMBIOS
+config DELL_WMI_SMBIOS
 	tristate "Dell WMI SMBIOS calling interface"
 	depends on ACPI_WMI
 	---help---
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec1751461..b127a3bc1fab 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -11,11 +11,11 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 obj-$(CONFIG_MSI_LAPTOP)	+= msi-laptop.o
 obj-$(CONFIG_ACPI_CMPC)		+= classmate-laptop.o
 obj-$(CONFIG_COMPAL_LAPTOP)	+= compal-laptop.o
-obj-$(CONFIG_DELL_SMBIOS)	+= dell-smbios.o
 obj-$(CONFIG_DELL_LAPTOP)	+= dell-laptop.o
 obj-$(CONFIG_DELL_WMI)		+= dell-wmi.o
 obj-$(CONFIG_DELL_WMI_AIO)	+= dell-wmi-aio.o
 obj-$(CONFIG_DELL_WMI_LED)	+= dell-wmi-led.o
+obj-$(CONFIG_DELL_WMI_SMBIOS)	+= dell-wmi-smbios.o
 obj-$(CONFIG_DELL_SMO8800)	+= dell-smo8800.o
 obj-$(CONFIG_DELL_RBTN)		+= dell-rbtn.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index f42159fd2031..bf569ea93e9d 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -33,7 +33,7 @@
 #include <linux/seq_file.h>
 #include <acpi/video.h>
 #include "dell-rbtn.h"
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 #define BRIGHTNESS_TOKEN 0x7d
 #define KBD_LED_OFF_TOKEN 0x01E1
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
similarity index 92%
rename from drivers/platform/x86/dell-smbios.c
rename to drivers/platform/x86/dell-wmi-smbios.c
index c06262a89169..7f896701fb7b 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -21,7 +21,7 @@
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/wmi.h>
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 #define DELL_WMI_SMBIOS_GUID "A80593CE-A997-11DA-B012-B622A1EF5492"
 
@@ -189,7 +189,7 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
-static int dell_smbios_probe(struct wmi_device *wdev)
+static int dell_wmi_smbios_probe(struct wmi_device *wdev)
 {
 	int ret;
 
@@ -213,27 +213,27 @@ static int dell_smbios_probe(struct wmi_device *wdev)
 	return ret;
 }
 
-static int dell_smbios_remove(struct wmi_device *wdev)
+static int dell_wmi_smbios_remove(struct wmi_device *wdev)
 {
 	kfree(da_tokens);
 	free_page((unsigned long)buffer);
 	return 0;
 }
 
-static const struct wmi_device_id dell_smbios_id_table[] = {
+static const struct wmi_device_id dell_wmi_smbios_id_table[] = {
 	{ .guid_string = DELL_WMI_SMBIOS_GUID },
 	{ },
 };
 
-static struct wmi_driver dell_smbios_driver = {
+static struct wmi_driver dell_wmi_smbios_driver = {
 	.driver = {
-		.name = "dell-smbios",
+		.name = "dell-wmi-smbios",
 	},
-	.probe = dell_smbios_probe,
-	.remove = dell_smbios_remove,
-	.id_table = dell_smbios_id_table,
+	.probe = dell_wmi_smbios_probe,
+	.remove = dell_wmi_smbios_remove,
+	.id_table = dell_wmi_smbios_id_table,
 };
-module_wmi_driver(dell_smbios_driver);
+module_wmi_driver(dell_wmi_smbios_driver);
 
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-wmi-smbios.h
similarity index 96%
rename from drivers/platform/x86/dell-smbios.h
rename to drivers/platform/x86/dell-wmi-smbios.h
index e1e29697b362..e6e9990bb2b7 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-wmi-smbios.h
@@ -14,8 +14,8 @@
  *  published by the Free Software Foundation.
  */
 
-#ifndef _DELL_SMBIOS_H_
-#define _DELL_SMBIOS_H_
+#ifndef _DELL_WMI_SMBIOS_H_
+#define _DELL_WMI_SMBIOS_H_
 
 struct notifier_block;
 
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 1fbef560ca67..e8b4d412eabc 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -38,7 +38,7 @@
 #include <linux/dmi.h>
 #include <linux/wmi.h>
 #include <acpi/video.h>
-#include "dell-smbios.h"
+#include "dell-wmi-smbios.h"
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1739151

On Thursday 21 September 2017 08:57:05 Mario Limonciello wrote:
> The existing way that the dell-smbios helper module and associated
> other drivers (dell-laptop, dell-wmi) communicate with the platform
> really isn't secure.  It requires creating a buffer in physical
> DMA32 memory space and passing that to the platform via SMM.
> 
> Since the platform got a physical memory pointer, you've just got
> to trust that the platform has only modified (and accessed) memory
> within that buffer.

And what is the problem? The whole memory management is done by kernel
itself, so you already need to trust it.

> Dell Platform designers recognize this security risk and offer a
> safer way to communicate with the platform over ACPI.  This is
> in turn exposed via a WMI interface to the OS.

Hm... I cannot understand how some proprietary ACPI bytecode interpreted
by kernel can be safer as kernel code itself.

Can you describe more details about this security risk?

> When communicating over WMI-ACPI the communication doesn't occur
> with physical memory pointers.  When the ASL is invoked, the fixed
> length ACPI buffer is copied to a small operating region.  The ASL
> will invoke the SMI, and SMM will only have access to this operating
> region.  When the ASL returns the buffer is copied back for the OS
> to process.

If problem is in current kernel implementation, then it can be fixed.

I'm not against using new WMI communication, but I cannot understand how
kernel code itself is less safer as some other code which is interpreted
by kernel. It does not make sense for me.

> This method of communication should also deprecate the usage of the
> dcdbas kernel module and software dependent upon it's interface.
> Instead offer a syfs interface for communicating with this ASL
> method to allow userspace to use instead.
> 
> To faciliate that needs for userspace and kernel space this patch
> series introduces a generic way for WMI drivers to be able to
> create character devices through the WMI bus when desired.
> Requiring WMI drivers to explictly ask for this functionality will
> act as an effective vendor whitelist.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1739166

Hi Pali,

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, September 25, 2017 12:14 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; platform-driver-
> x86@vger.kernel.org; quasisec@google.com
> Subject: Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI
> 
> On Thursday 21 September 2017 08:57:05 Mario Limonciello wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> 
> And what is the problem? The whole memory management is done by kernel
> itself, so you already need to trust it.

There's a lot of ifs, but it's not that crazy of a scenario.

The problem is that if a malicious payload was delivered to the platform
and exercised a vulnerability in the platform code that payload could 
potentially modify memory that it wasn't intended to modify and the OS
would not be aware as operating in SMM.

> 
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> 
> Hm... I cannot understand how some proprietary ACPI bytecode interpreted
> by kernel can be safer as kernel code itself.
> 

Inherently ACPI can only operate on operation regions and not physical memory.
Data passed into ACPI needs to be copied to an operation region for any ACPI
calls to use it.

Furthermore you can decompile the ASL and audit, you can't do this with direct
SMI/SMM.

> Can you describe more details about this security risk?
> 
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> 
> If problem is in current kernel implementation, then it can be fixed.
> 
>
> I'm not against using new WMI communication, but I cannot understand how
> kernel code itself is less safer as some other code which is interpreted
> by kernel. It does not make sense for me.
> 

Well we're talking hypotheticals here in the way things work.
There aren't necessarily problems with the current implementation.

Also, I didn't already mention this explicitly but I've alluded it to it;
Dell is deprecating that interface.  I can't say when, but it will stop working
on some new hardware at some point.

That's the other reason why I'm pushing for the new communication path
now.

> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a syfs interface for communicating with this ASL
> > method to allow userspace to use instead.
> >
> > To faciliate that needs for userspace and kernel space this patch
> > series introduces a generic way for WMI drivers to be able to
> > create character devices through the WMI bus when desired.
> > Requiring WMI drivers to explictly ask for this functionality will
> > act as an effective vendor whitelist.
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1739171

On Monday 25 September 2017 16:32:52 Mario.Limonciello@dell.com wrote:
> Hi Pali,
> 
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Monday, September 25, 2017 12:14 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; platform-driver-
> > x86@vger.kernel.org; quasisec@google.com
> > Subject: Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI
> > 
> > On Thursday 21 September 2017 08:57:05 Mario Limonciello wrote:
> > > The existing way that the dell-smbios helper module and associated
> > > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > > really isn't secure.  It requires creating a buffer in physical
> > > DMA32 memory space and passing that to the platform via SMM.
> > >
> > > Since the platform got a physical memory pointer, you've just got
> > > to trust that the platform has only modified (and accessed) memory
> > > within that buffer.
> > 
> > And what is the problem? The whole memory management is done by kernel
> > itself, so you already need to trust it.
> 
> There's a lot of ifs, but it's not that crazy of a scenario.
> 
> The problem is that if a malicious payload was delivered to the platform
> and exercised a vulnerability in the platform code that payload could 
> potentially modify memory that it wasn't intended to modify and the OS
> would not be aware as operating in SMM.
> 
> > 
> > > Dell Platform designers recognize this security risk and offer a
> > > safer way to communicate with the platform over ACPI.  This is
> > > in turn exposed via a WMI interface to the OS.
> > 
> > Hm... I cannot understand how some proprietary ACPI bytecode interpreted
> > by kernel can be safer as kernel code itself.
> > 
> 
> Inherently ACPI can only operate on operation regions and not physical memory.
> Data passed into ACPI needs to be copied to an operation region for any ACPI
> calls to use it.

But operation regions access is implemented by ACPI interpreter, which
is again kernel code.

> Furthermore you can decompile the ASL and audit, you can't do this with direct
> SMI/SMM.
> 
> > Can you describe more details about this security risk?
> > 
> > > When communicating over WMI-ACPI the communication doesn't occur
> > > with physical memory pointers.  When the ASL is invoked, the fixed
> > > length ACPI buffer is copied to a small operating region.  The ASL
> > > will invoke the SMI, and SMM will only have access to this operating
> > > region.  When the ASL returns the buffer is copied back for the OS
> > > to process.
> > 
> > If problem is in current kernel implementation, then it can be fixed.
> > 
> >
> > I'm not against using new WMI communication, but I cannot understand how
> > kernel code itself is less safer as some other code which is interpreted
> > by kernel. It does not make sense for me.
> > 
> 
> Well we're talking hypotheticals here in the way things work.
> There aren't necessarily problems with the current implementation.

Ok.

> Also, I didn't already mention this explicitly but I've alluded it to it;
> Dell is deprecating that interface.  I can't say when, but it will stop working
> on some new hardware at some point.
> 
> That's the other reason why I'm pushing for the new communication path
> now.

Ok, as I wrote I'm not against new communication method and specially
now, when you confirmed that in future new machines would not support
"old" method...

... but old communication method should stay there for older machines. I
do not think it would be hard to have both implementations in kernel and
choosing that which is supported on current machine.

> > > This method of communication should also deprecate the usage of the
> > > dcdbas kernel module and software dependent upon it's interface.
> > > Instead offer a syfs interface for communicating with this ASL
> > > method to allow userspace to use instead.
> > >
> > > To faciliate that needs for userspace and kernel space this patch
> > > series introduces a generic way for WMI drivers to be able to
> > > create character devices through the WMI bus when desired.
> > > Requiring WMI drivers to explictly ask for this functionality will
> > > act as an effective vendor whitelist.
> > 
> > --
> > Pali Rohár
> > pali.rohar@gmail.com

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1739235

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, September 25, 2017 12:49 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; quasisec@google.com
> Subject: Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI
> 
> On Monday 25 September 2017 16:32:52 Mario.Limonciello@dell.com wrote:
> > Hi Pali,
> >
> > > -----Original Message-----
> > > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > > Sent: Monday, September 25, 2017 12:14 PM
> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; platform-
> driver-
> > > x86@vger.kernel.org; quasisec@google.com
> > > Subject: Re: [PATCH 00/12] Introduce support for Dell SMBIOS over WMI
> > >
> > > On Thursday 21 September 2017 08:57:05 Mario Limonciello wrote:
> > > > The existing way that the dell-smbios helper module and associated
> > > > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > > > really isn't secure.  It requires creating a buffer in physical
> > > > DMA32 memory space and passing that to the platform via SMM.
> > > >
> > > > Since the platform got a physical memory pointer, you've just got
> > > > to trust that the platform has only modified (and accessed) memory
> > > > within that buffer.
> > >
> > > And what is the problem? The whole memory management is done by kernel
> > > itself, so you already need to trust it.
> >
> > There's a lot of ifs, but it's not that crazy of a scenario.
> >
> > The problem is that if a malicious payload was delivered to the platform
> > and exercised a vulnerability in the platform code that payload could
> > potentially modify memory that it wasn't intended to modify and the OS
> > would not be aware as operating in SMM.
> >
> > >
> > > > Dell Platform designers recognize this security risk and offer a
> > > > safer way to communicate with the platform over ACPI.  This is
> > > > in turn exposed via a WMI interface to the OS.
> > >
> > > Hm... I cannot understand how some proprietary ACPI bytecode interpreted
> > > by kernel can be safer as kernel code itself.
> > >
> >
> > Inherently ACPI can only operate on operation regions and not physical memory.
> > Data passed into ACPI needs to be copied to an operation region for any ACPI
> > calls to use it.
> 
> But operation regions access is implemented by ACPI interpreter, which
> is again kernel code.

So isn't that making my point?
* Kernel can control operation region accessibility.  SMM can't operate outside
of this region.
* Direct SMI gives platform access to everything < 4G, kernel can't control this.

> 
> > Furthermore you can decompile the ASL and audit, you can't do this with direct
> > SMI/SMM.
> >
> > > Can you describe more details about this security risk?
> > >
> > > > When communicating over WMI-ACPI the communication doesn't occur
> > > > with physical memory pointers.  When the ASL is invoked, the fixed
> > > > length ACPI buffer is copied to a small operating region.  The ASL
> > > > will invoke the SMI, and SMM will only have access to this operating
> > > > region.  When the ASL returns the buffer is copied back for the OS
> > > > to process.
> > >
> > > If problem is in current kernel implementation, then it can be fixed.
> > >
> > >
> > > I'm not against using new WMI communication, but I cannot understand how
> > > kernel code itself is less safer as some other code which is interpreted
> > > by kernel. It does not make sense for me.
> > >
> >
> > Well we're talking hypotheticals here in the way things work.
> > There aren't necessarily problems with the current implementation.
> 
> Ok.
> 
> > Also, I didn't already mention this explicitly but I've alluded it to it;
> > Dell is deprecating that interface.  I can't say when, but it will stop working
> > on some new hardware at some point.
> >
> > That's the other reason why I'm pushing for the new communication path
> > now.
> 
> Ok, as I wrote I'm not against new communication method and specially
> now, when you confirmed that in future new machines would not support
> "old" method...
> 
> ... but old communication method should stay there for older machines. I
> do not think it would be hard to have both implementations in kernel and
> choosing that which is supported on current machine.

The WMI interface has been around for at least 10 years.  I think the number
of machines still running on the older implementation only is very small.

There is however a bit that will set the availability of this interface.  I'll rework
my patches to offer both WMI and legacy SMI approach based upon that
presence of the bit.

> 
> > > > This method of communication should also deprecate the usage of the
> > > > dcdbas kernel module and software dependent upon it's interface.
> > > > Instead offer a syfs interface for communicating with this ASL
> > > > method to allow userspace to use instead.
> > > >
> > > > To faciliate that needs for userspace and kernel space this patch
> > > > series introduces a generic way for WMI drivers to be able to
> > > > create character devices through the WMI bus when desired.
> > > > Requiring WMI drivers to explictly ask for this functionality will
> > > > act as an effective vendor whitelist.
> > >
> > > --
> > > Pali Rohár
> > > pali.rohar@gmail.com
> 
> --
> Pali Rohár
> pali.rohar@gmail.com

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web