Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Daniel Gutson <daniel.gutson@eclypsium.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	Richard Hughes <hughsient@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Derek Kiernan <derek.kiernan@xilinx.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Alex Bazhaniuk <alex@eclypsium.com>
Subject: Re: [PATCH 2/2] Platform integrity information in sysfs (version 9)
Date: Fri, 2 Oct 2020 15:43:31 +0200
Message-ID: <20201002134331.GA3418160@kroah.com> (raw)
In-Reply-To: <20200930163714.12879-3-daniel.gutson@eclypsium.com>

On Wed, Sep 30, 2020 at 01:37:14PM -0300, Daniel Gutson wrote:
> This patch exports the BIOS Write Enable (bioswe), BIOS
> Lock Enable (biosle), and the SMM BIOS Write Protect (SMM_BIOSWP) fields of
> the BIOS Control register using the platform-integrity misc kernel module.
> The idea is to keep adding more flags, not only from the BC but also from
> other registers in following versions.
> 
> The goal is that the attributes are avilable to fwupd when SecureBoot
> is turned on.
> 
> Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>

The subject line doesn't match what this patch does, please fix that up.

But there are more core issues in this patch:

> 
> ---
>  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
>  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 75 ++++++++++++++-
>  .../spi-nor/controllers/intel-spi-platform.c  |  2 +-
>  drivers/mtd/spi-nor/controllers/intel-spi.c   | 91 ++++++++++++++++++-
>  drivers/mtd/spi-nor/controllers/intel-spi.h   |  9 +-
>  5 files changed, 171 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
> index 5c0e0ec2e6d1..e7eaef506fc2 100644
> --- a/drivers/mtd/spi-nor/controllers/Kconfig
> +++ b/drivers/mtd/spi-nor/controllers/Kconfig
> @@ -29,6 +29,7 @@ config SPI_NXP_SPIFI
>  
>  config SPI_INTEL_SPI
>  	tristate
> +	depends on PLATFORM_INTEGRITY_DATA
>  
>  config SPI_INTEL_SPI_PCI
>  	tristate "Intel PCH/PCU SPI flash PCI driver (DANGEROUS)"
> diff --git a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> index c72aa1ab71ad..644b1a6091dc 100644
> --- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> +++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
> @@ -10,11 +10,19 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> +#include <linux/platform-integrity.h>
>  
>  #include "intel-spi.h"
>  
>  #define BCR		0xdc
>  #define BCR_WPD		BIT(0)
> +#define BCR_BLE		BIT(1)
> +#define BCR_SMM_BWP	BIT(5)
> +
> +struct cnl_spi_attr {
> +	struct device_attribute dev_attr;
> +	u32 mask;
> +};
>  
>  static const struct intel_spi_boardinfo bxt_info = {
>  	.type = INTEL_SPI_BXT,
> @@ -24,6 +32,70 @@ static const struct intel_spi_boardinfo cnl_info = {
>  	.type = INTEL_SPI_CNL,
>  };
>  
> +#ifdef CONFIG_PLATFORM_INTEGRITY_DATA
> +static ssize_t intel_spi_cnl_spi_attr_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf, u32 mask)
> +{
> +	u32 bcr;
> +
> +	if (dev->parent == NULL)
> +		return -EIO;
> +
> +	if (pci_read_config_dword(container_of(dev->parent, struct pci_dev, dev),
> +				  BCR, &bcr) != PCIBIOS_SUCCESSFUL)
> +		return -EIO;
> +
> +	return sprintf(buf, "%d\n", (int)!!(bcr & mask));
> +}
> +
> +static ssize_t bioswe_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_WPD);
> +}
> +static DEVICE_ATTR_RO(bioswe);
> +
> +static ssize_t biosle_show(struct device *dev, struct device_attribute *attr,
> +			   char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_BLE);
> +}
> +static DEVICE_ATTR_RO(biosle);
> +
> +static ssize_t smm_bioswp_show(struct device *dev, struct device_attribute *attr,
> +			       char *buf)
> +{
> +	return intel_spi_cnl_spi_attr_show(dev, attr, buf, BCR_SMM_BWP);
> +}
> +static DEVICE_ATTR_RO(smm_bioswp);
> +
> +static struct attribute *cnl_attrs[] = {
> +	&dev_attr_bioswe.attr,
> +	&dev_attr_biosle.attr,
> +	&dev_attr_smm_bioswp.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(cnl);

If you are forcing the driver to create the groups, then you are forcing
us to audit each driver and verify that the files are the same name and
such.  Put the files in the "common" code please, and if you really need
a way to get the data out, make that a callback or something.

Also, this name "platform integrity" is really really generic, while in
reality you are describing something very specific.  Are you sure you
want that?

thanks,

greg k-h

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 16:37 [PATCH 0/2] " Daniel Gutson
2020-09-30 16:37 ` [PATCH 1/2] " Daniel Gutson
2020-09-30 16:37 ` [PATCH 2/2] " Daniel Gutson
2020-10-02 13:43   ` Greg Kroah-Hartman [this message]
2020-10-21 19:55     ` Daniel Gutson
2020-11-10 14:07       ` Daniel Gutson
2020-10-04  4:01   ` Randy Dunlap
2020-10-22 12:08     ` Daniel Gutson
2020-09-30 16:42 ` [PATCH 0/2] " Daniel Gutson
  -- strict thread matches above, loose matches on Subject: below --
2020-09-30 13:51 [PATCH 2/2] " Daniel Gutson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201002134331.GA3418160@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alex@eclypsium.com \
    --cc=arnd@arndb.de \
    --cc=daniel.gutson@eclypsium.com \
    --cc=derek.kiernan@xilinx.com \
    --cc=hughsient@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=tudor.ambarus@microchip.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git