linux-kernel - Re: [PATCH v5 10/14] platform/x86: dell-smbios: add filtering capability for requests

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]

Date:   Sat, 7 Oct 2017 09:43:33 +0200
From:   Greg KH <greg@...ah.com>
To:     Mario Limonciello <mario.limonciello@...l.com>
Cc:     dvhart@...radead.org, 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
Subject: Re: [PATCH v5 10/14] platform/x86: dell-smbios: add filtering
 capability for requests

On Fri, Oct 06, 2017 at 11:59:54PM -0500, Mario Limonciello wrote:
> There are some categories of tokens and SMBIOS calls that it makes
> sense to protect userspace from accessing.  These are calls that
> may write to one time use fields or activate hardware debugging
> capabilities.  They are not intended for general purpose use.
> 
> This same functionality may be be later extended to also intercept
> calls that may cause kernel functionality to get out of sync if
> the same functions are used by other drivers.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@...l.com>
> ---
>  drivers/platform/x86/dell-smbios.c | 76 ++++++++++++++++++++++++++++++++++++++
>  drivers/platform/x86/dell-smbios.h |  2 +
>  2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index 2f90ba5346bc..d1908f159be3 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -32,6 +32,7 @@ struct calling_interface_structure {
>  	struct calling_interface_token tokens[];
>  } __packed;
>  
> +static u32 da_supported_commands;
>  static int da_command_address;
>  static int da_command_code;
>  static int da_num_tokens;
> @@ -45,6 +46,14 @@ struct smbios_device {
>  	int (*call_fn)(struct calling_interface_buffer *);
>  };
>  
> +static u32 token_black[] = {
> +	0x0175, 0x0176, 0x0195, 0x0196, 0x0197, 0x01DC, 0x01DD, 0x027D, 0x027E,
> +	0x027F, 0x0280, 0x0281,	0x0282, 0x0283, 0x0284, 0x02E3, 0x02FF, 0x0300,
> +	0x0301, 0x0302, 0x0325, 0x0326, 0x0332, 0x0333,	0x0334, 0x0335, 0x0350,
> +	0x0363, 0x0368, 0x03F6, 0x03F7, 0x049E, 0x049F, 0x04A0, 0x04A1, 0x04A2,
> +	0x04A3, 0x04E6, 0x04E7, 0x9000, 0x9001
> +};

Any hint as to what these values represent?

>  static LIST_HEAD(smbios_device_list);
>  
>  void dell_smbios_get_smm_address(int *address, int *code)
> @@ -104,6 +113,65 @@ void dell_smbios_unregister_device(struct device *d)
>  }
>  EXPORT_SYMBOL_GPL(dell_smbios_unregister_device);
>  
> +int dell_smbios_call_filter(struct device *d,
> +			    struct calling_interface_buffer *buffer)
> +{
> +	int i;
> +	int j;
> +	u32 t;
> +
> +	/* can't make calls over 30 */
> +	if (buffer->class > 30) {
> +		dev_dbg(d, "buffer->class too big: %d\n", buffer->class);
> +		return -EINVAL;
> +	}
> +
> +	/* supported calls on the particular system */
> +	if (!(da_supported_commands & (1 << buffer->class))) {
> +		dev_dbg(d, "invalid command, supported commands: 0x%8x\n",
> +			da_supported_commands);
> +		return -EINVAL;
> +	}
> +
> +	/* diagonstics, debugging information or write once  */
> +	if ((buffer->class == 01 && buffer->select == 07) ||
> +	    (buffer->class == 06 && buffer->select == 05) ||
> +	    (buffer->class == 11 && buffer->select == 03) ||
> +	    (buffer->class == 11 && buffer->select == 07) ||
> +	    (buffer->class == 11 && buffer->select == 11) ||
> +	     buffer->class == 19) {

A structure of class/select that is not allowed might be easier to
maintain over time, right?

> +		dev_dbg(d, "blacklisted command: %d/%d\n",
> +			buffer->class, buffer->select);
> +		return -EINVAL;
> +	}
> +
> +	/* reading/writing tokens*/
> +	if ((buffer->class == 0 && buffer->select < 3) ||
> +	    (buffer->class == 1 && buffer->select < 3)) {
> +		for (i = 0; i < da_num_tokens; i++) {
> +			if (da_tokens[i].location != buffer->input[0])
> +				continue;
> +			/*blacklist reading and writing these */

"/* " ???

thanks,

greg k-h