linux-kernel - RE: [PATCH v4 09/14] platform/x86: dell-smbios: Introduce dispatcher for SMM calls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2bc391409724010b96170f9d206f622@ausx13mpc120.AMER.DELL.COM>
Date:   Thu, 5 Oct 2017 15:04:47 +0000
From:   <Mario.Limonciello@...l.com>
To:     <dvhart@...radead.org>
CC:     <andy.shevchenko@...il.com>, <linux-kernel@...r.kernel.org>,
        <platform-driver-x86@...r.kernel.org>, <luto@...nel.org>,
        <quasisec@...gle.com>, <pali.rohar@...il.com>, <rjw@...ysocki.net>,
        <mjg59@...gle.com>, <hch@....de>, <greg@...ah.com>
Subject: RE: [PATCH v4 09/14] platform/x86: dell-smbios: Introduce dispatcher
 for SMM calls

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@...radead.org]
> Sent: Wednesday, October 4, 2017 8:58 PM
> To: Limonciello, Mario <Mario_Limonciello@...l.com>
> Cc: Andy Shevchenko <andy.shevchenko@...il.com>; LKML <linux-
> kernel@...r.kernel.org>; platform-driver-x86@...r.kernel.org; Andy Lutomirski
> <luto@...nel.org>; quasisec@...gle.com; pali.rohar@...il.com;
> rjw@...ysocki.net; mjg59@...gle.com; hch@....de; Greg KH
> <greg@...ah.com>
> Subject: Re: [PATCH v4 09/14] platform/x86: dell-smbios: Introduce dispatcher for
> SMM calls
> 
> On Wed, Oct 04, 2017 at 05:48:35PM -0500, Mario Limonciello wrote:
> > This splits up the dell-smbios driver into two drivers:
> > * dell-smbios
> > * dell-smbios-smm
> >
> > dell-smbios can operate with multiple different dispatcher drivers to
> > perform SMBIOS operations.
> >
> > Also modify the interface that dell-laptop and dell-wmi use align to this
> > model more closely.  Rather than a single global buffer being allocated
> > for all drivers, each driver will allocate and be responsible for it's own
> > buffer. The pointer will be passed to the calling function and each
> > dispatcher driver will then internally copy it to the proper location to
> > perform it's call.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@...l.com>
> > ---
> >  MAINTAINERS                            |   6 ++
> >  drivers/platform/x86/Kconfig           |  17 ++-
> >  drivers/platform/x86/Makefile          |   1 +
> >  drivers/platform/x86/dell-laptop.c     | 191 ++++++++++++++++-----------------
> >  drivers/platform/x86/dell-smbios-smm.c | 136 +++++++++++++++++++++++
> >  drivers/platform/x86/dell-smbios-smm.h |  22 ++++
> >  drivers/platform/x86/dell-smbios.c     | 120 +++++++++++++--------
> >  drivers/platform/x86/dell-smbios.h     |  13 ++-
> >  drivers/platform/x86/dell-wmi.c        |   8 +-
> >  9 files changed, 361 insertions(+), 153 deletions(-)
> >  create mode 100644 drivers/platform/x86/dell-smbios-smm.c
> >  create mode 100644 drivers/platform/x86/dell-smbios-smm.h
> >
> ...
> 
> > +config DELL_SMBIOS_SMM
> > +       tristate "Dell SMBIOS calling interface (SMM implementation)"
> > +       depends on DCDBAS
> > +       default DCDBAS
> > +       select DELL_SMBIOS
> > +       ---help---
> > +       This provides an implementation for the Dell SMBIOS calling interface
> > +       communicated over ACPI-WMI.
> 
> Not over ACPI-WMI... over SMM.... right?

Yep, copy paste error.

> 
> 
> 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-
> laptop.c
> > index f42159fd2031..c648bbfcc394 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -85,6 +85,7 @@ static struct platform_driver platform_driver = {
> >  	}
> >  };
> >
> > +static struct calling_interface_buffer *buffer;
> >  static struct platform_device *platform_device;
> >  static struct backlight_device *dell_backlight_device;
> >  static struct rfkill *wifi_rfkill;
> > @@ -405,7 +406,6 @@ static const struct dmi_system_id dell_quirks[]
> __initconst = {
> >
> >  static int dell_rfkill_set(void *data, bool blocked)
> >  {
> > -	struct calling_interface_buffer *buffer;
> >  	int disable = blocked ? 1 : 0;
> >  	unsigned long radio = (unsigned long)data;
> >  	int hwswitch_bit = (unsigned long)data - 1;
> > @@ -413,19 +413,21 @@ static int dell_rfkill_set(void *data, bool blocked)
> >  	int status;
> >  	int ret;
> >
> > -	buffer = dell_smbios_get_buffer();
> > -
> > -	dell_smbios_send_request(17, 11);
> > +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> > +	buffer->class = 17;
> > +	buffer->select = 11;
> 
> Please move these three lines into a function, it's used far far too
> often to be open coded. The dell_smbios_send_request is a reasonable
> wrapper which you could just define here as well. This would minimize
> the change to the driver.
> 
OK I'll clean this up and make some functions.