[PATCH] Platform lockdown information in SYSFS

[PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

This patch exports information about the platform lockdown
firmware configuration in the sysfs filesystem.
In this initial patch, I include some configuration attributes
for the system SPI chip.

This initial version exports the BIOS Write Enable (bioswe),
BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
fields of the Bios Control register. 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.

The patch provides a new misc driver, as proposed in the previous patch,
that provides a registration function for HW Driver devices to register
class_attributes.
In this case, the intel SPI flash chip (intel-spi) registers three
class_attributes corresponding to the fields mentioned above.

Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
---
 .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
 MAINTAINERS                                   |  7 +++
 drivers/misc/Kconfig                          |  9 +++
 drivers/misc/Makefile                         |  1 +
 drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
 drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
 .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
 drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
 .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
 9 files changed, 228 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
 create mode 100644 drivers/misc/platform-lockdown-attrs.c
 create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h

diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
new file mode 100644
index 000000000000..6034d6cbefac
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
@@ -0,0 +1,23 @@
+What:		/sys/class/platform-lockdown/bioswe
+Date:		July 2020
+KernelVersion:	5.8.0
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set BIOS Write Enable.
+		0: writes disabled, 1: writes enabled.
+Users:		https://github.com/fwupd/fwupd
+
+What:		/sys/class/platform-lockdown/ble
+Date:		July 2020
+KernelVersion:	5.8.0
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set Bios Lock Enable.
+		0: SMM lock disabled, 1: SMM lock enabled.
+Users:		https://github.com/fwupd/fwupd
+
+What:		/sys/class/platform-lockdown/smm_bwp
+Date:		July 2020
+KernelVersion:	5.8.0
+Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
+Description:	If the system firmware set SMM Bios Write Protect.
+		0: writes disabled unless in SMM, 1: writes enabled.
+Users:		https://github.com/fwupd/fwupd
diff --git a/MAINTAINERS b/MAINTAINERS
index f0569cf304ca..771ed1693d28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13608,6 +13608,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
 F:	drivers/iio/chemical/pms7003.c
 
+PLATFORM LOCKDOWN ATTRIBUTES MODULE
+M:	Daniel Gutson <daniel.gutson@eclypsium.com>
+S:	Supported
+F:	Documentation/ABI/sysfs-class-platform-lockdown
+F:	drivers/misc/platform-lockdown-attrs.c
+F:	include/linux/platform_data/platform-lockdown-attrs.h
+
 PLX DMA DRIVER
 M:	Logan Gunthorpe <logang@deltatee.com>
 S:	Maintained
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e1b1ba5e2b92..058d4ba3cefd 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -456,6 +456,15 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config PLATFORM_LOCKDOWN_ATTRS
+	tristate "Platform lockdown information in the SYSFS"
+	depends on SYSFS
+	help
+	  This kernel module is a helper driver to provide information about
+	  platform lockdown settings and configuration.
+	  This module is used by other device drivers -such as the intel-spi-
+	  to publish the information in /sys/class/platform-lockdown.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c7bd01ac6291..e29b45c564f9 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
+obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS)	+= platform-lockdown-attrs.o
diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c
new file mode 100644
index 000000000000..d08b3d895064
--- /dev/null
+++ b/drivers/misc/platform-lockdown-attrs.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Platform lockdown attributes kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/platform_data/platform-lockdown-attrs.h>
+
+static struct class platform_lockdown_class = {
+	.name	= "platform-lockdown",
+	.owner	= THIS_MODULE,
+};
+
+int register_platform_lockdown_attribute(
+	struct class_attribute *lockdown_attribute)
+{
+	/* attempt to create the file: */
+	return class_create_file(&platform_lockdown_class,
+				   lockdown_attribute);
+}
+EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute);
+
+void unregister_platform_lockdown_attribute(
+	struct class_attribute *lockdown_attribute)
+{
+	class_remove_file(&platform_lockdown_class,
+				   lockdown_attribute);
+}
+EXPORT_SYMBOL_GPL(unregister_platform_lockdown_attribute);
+
+static int __init platform_lockdown_attrs_init(void)
+{
+	int status;
+
+	status = class_register(&platform_lockdown_class);
+	if (status < 0)
+		return status;
+
+	return 0;
+}
+
+static void __exit platform_lockdown_attrs_exit(void)
+{
+	class_unregister(&platform_lockdown_class);
+}
+
+module_init(platform_lockdown_attrs_init);
+module_exit(platform_lockdown_attrs_exit);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Daniel Gutson <daniel.gutson@eclypsium.com>");
diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
index d89a5ea9446a..3c03188cc30b 100644
--- a/drivers/mtd/spi-nor/controllers/Kconfig
+++ b/drivers/mtd/spi-nor/controllers/Kconfig
@@ -40,6 +40,7 @@ config SPI_NXP_SPIFI
 
 config SPI_INTEL_SPI
 	tristate
+	select PLATFORM_LOCKDOWN_ATTRS
 
 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 81329f680bec..d99b0b6fe432 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi-pci.c
@@ -10,11 +10,20 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/platform-lockdown-attrs.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 class_attribute class_attr;
+	struct pci_dev *pdev;
+	u32 mask;
+};
 
 static const struct intel_spi_boardinfo bxt_info = {
 	.type = INTEL_SPI_BXT,
@@ -24,6 +33,40 @@ static const struct intel_spi_boardinfo cnl_info = {
 	.type = INTEL_SPI_CNL,
 };
 
+static ssize_t cnl_spi_attr_show(struct class *class,
+	struct class_attribute *attr, char *buf)
+{
+	u32 bcr;
+	struct cnl_spi_attr *cnl_spi_attr = container_of(attr,
+		struct cnl_spi_attr, class_attr);
+
+	if (pci_read_config_dword(cnl_spi_attr->pdev,
+		BCR, &bcr) != PCIBIOS_SUCCESSFUL)
+		return -EIO;
+
+	return sprintf(buf, "%d\n", (int)!!(bcr & cnl_spi_attr->mask));
+}
+
+#define CNL_SPI_ATTR(_name, _mask)					\
+static struct cnl_spi_attr cnl_##_name##_attr = {			\
+	.class_attr = __ATTR(_name, 0444, cnl_spi_attr_show, NULL),	\
+	.mask = _mask							\
+}
+
+CNL_SPI_ATTR(bioswe, BCR_WPD);
+CNL_SPI_ATTR(ble, BCR_BLE);
+CNL_SPI_ATTR(smm_bwp, BCR_SMM_BWP);
+
+static void register_platform_lockdown_attributes(struct pci_dev *pdev)
+{
+	cnl_bioswe_attr.pdev = pdev;
+	cnl_ble_attr.pdev = pdev;
+	cnl_smm_bwp_attr.pdev = pdev;
+	register_platform_lockdown_attribute(&cnl_bioswe_attr.class_attr);
+	register_platform_lockdown_attribute(&cnl_ble_attr.class_attr);
+	register_platform_lockdown_attribute(&cnl_smm_bwp_attr.class_attr);
+}
+
 static int intel_spi_pci_probe(struct pci_dev *pdev,
 			       const struct pci_device_id *id)
 {
@@ -50,6 +93,8 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
 	}
 	info->writeable = !!(bcr & BCR_WPD);
 
+	register_platform_lockdown_attributes(pdev);
+
 	ispi = intel_spi_probe(&pdev->dev, &pdev->resource[0], info);
 	if (IS_ERR(ispi))
 		return PTR_ERR(ispi);
@@ -60,6 +105,10 @@ static int intel_spi_pci_probe(struct pci_dev *pdev,
 
 static void intel_spi_pci_remove(struct pci_dev *pdev)
 {
+	unregister_platform_lockdown_attribute(&cnl_bioswe_attr.class_attr);
+	unregister_platform_lockdown_attribute(&cnl_ble_attr.class_attr);
+	unregister_platform_lockdown_attribute(&cnl_smm_bwp_attr.class_attr);
+
 	intel_spi_remove(pci_get_drvdata(pdev));
 }
 
diff --git a/drivers/mtd/spi-nor/controllers/intel-spi.c b/drivers/mtd/spi-nor/controllers/intel-spi.c
index 61d2a0ad2131..13397dd9f89f 100644
--- a/drivers/mtd/spi-nor/controllers/intel-spi.c
+++ b/drivers/mtd/spi-nor/controllers/intel-spi.c
@@ -16,6 +16,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/platform_data/intel-spi.h>
+#include <linux/platform_data/platform-lockdown-attrs.h>
 
 #include "intel-spi.h"
 
@@ -95,6 +96,8 @@
 #define BYT_SSFSTS_CTL			0x90
 #define BYT_BCR				0xfc
 #define BYT_BCR_WPD			BIT(0)
+#define BYT_BCR_BLE			BIT(1)
+#define BYT_BCR_SMM_BWP			BIT(5)
 #define BYT_FREG_NUM			5
 #define BYT_PR_NUM			5
 
@@ -159,6 +162,12 @@ struct intel_spi {
 	u8 opcodes[8];
 };
 
+struct byt_spi_attr {
+	struct class_attribute class_attr;
+	struct intel_spi *ispi;
+	u32 mask;
+};
+
 static bool writeable;
 module_param(writeable, bool, 0);
 MODULE_PARM_DESC(writeable, "Enable write access to SPI flash chip (default=0)");
@@ -305,6 +314,56 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
 				  INTEL_SPI_TIMEOUT * 1000);
 }
 
+static ssize_t byt_spi_attr_show(struct class *class,
+	struct class_attribute *class_attr, char *buf)
+{
+	u32 bcr;
+	struct byt_spi_attr *byt_spi_attr = container_of(class_attr,
+		struct byt_spi_attr, class_attr);
+
+	bcr = readl(byt_spi_attr->ispi->base + BYT_BCR);
+	return sprintf(buf, "%d\n", (int)!!(bcr & byt_spi_attr->mask));
+}
+
+#define BYT_SPI_ATTR(_name, _mask)					\
+static struct byt_spi_attr byt_##_name##_attr = {			\
+	.class_attr = __ATTR(_name, 0444, byt_spi_attr_show, NULL),	\
+	.mask = _mask							\
+}
+
+BYT_SPI_ATTR(bioswe, BYT_BCR_WPD);
+BYT_SPI_ATTR(ble, BYT_BCR_BLE);
+BYT_SPI_ATTR(smm_bwp, BYT_BCR_SMM_BWP);
+
+static void register_platform_lockdown_attributes(struct intel_spi *ispi)
+{
+	switch (ispi->info->type) {
+	case INTEL_SPI_BYT:
+		byt_bioswe_attr.ispi = ispi;
+		byt_ble_attr.ispi = ispi;
+		byt_smm_bwp_attr.ispi = ispi;
+		register_platform_lockdown_attribute(&byt_bioswe_attr.class_attr);
+		register_platform_lockdown_attribute(&byt_ble_attr.class_attr);
+		register_platform_lockdown_attribute(&byt_smm_bwp_attr.class_attr);
+		break;
+	default:
+		break; /* TODO. not yet implemented. */
+	}
+}
+
+static void unregister_platform_lockdown_attributes(struct intel_spi *ispi)
+{
+	switch (ispi->info->type) {
+	case INTEL_SPI_BYT:
+		unregister_platform_lockdown_attribute(&byt_bioswe_attr.class_attr);
+		unregister_platform_lockdown_attribute(&byt_ble_attr.class_attr);
+		unregister_platform_lockdown_attribute(&byt_smm_bwp_attr.class_attr);
+		break;
+	default:
+		break; /* TODO. not yet implemented. */
+	}
+}
+
 static int intel_spi_init(struct intel_spi *ispi)
 {
 	u32 opmenu0, opmenu1, lvscc, uvscc, val;
@@ -422,6 +481,8 @@ static int intel_spi_init(struct intel_spi *ispi)
 
 	intel_spi_dump_regs(ispi);
 
+	register_platform_lockdown_attributes(ispi);
+
 	return 0;
 }
 
@@ -951,6 +1012,7 @@ EXPORT_SYMBOL_GPL(intel_spi_probe);
 
 int intel_spi_remove(struct intel_spi *ispi)
 {
+	unregister_platform_lockdown_attributes(ispi);
 	return mtd_device_unregister(&ispi->nor.mtd);
 }
 EXPORT_SYMBOL_GPL(intel_spi_remove);
diff --git a/include/linux/platform_data/platform-lockdown-attrs.h b/include/linux/platform_data/platform-lockdown-attrs.h
new file mode 100644
index 000000000000..f8ef7f3fd5b3
--- /dev/null
+++ b/include/linux/platform_data/platform-lockdown-attrs.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Platform lockdown attributes kernel module
+ *
+ * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
+ * Copyright (C) 2020 Eclypsium Inc.
+ */
+#ifndef PLATFORM_LOCKDOWN_ATTRS_H
+#define PLATFORM_LOCKDOWN_ATTRS_H
+
+#include <linux/device.h>
+
+extern int register_platform_lockdown_attribute(
+	struct class_attribute *lockdown_attribute);
+
+extern void unregister_platform_lockdown_attribute(
+	struct class_attribute *lockdown_attribute);
+
+#endif /* PLATFORM_LOCKDOWN_ATTRS_H */
-- 
2.25.1

Re: [PATCH] Platform lockdown information in SYSFS

[Randy Dunlap]

Hi,

Could we get some consistency in the use of "bios" vs. "Bios" vs. "BIOS", please.
BIOS is preferred IMO.

On 7/30/20 2:41 PM, Daniel Gutson wrote:
> 
> This initial version exports the BIOS Write Enable (bioswe),
> BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> fields of the Bios Control register. 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

                                      available

> is turned on.
> 
> The patch provides a new misc driver, as proposed in the previous patch,
> that provides a registration function for HW Driver devices to register
> class_attributes.
> In this case, the intel SPI flash chip (intel-spi) registers three
> class_attributes corresponding to the fields mentioned above.
> 
> Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> ---
>  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
>  MAINTAINERS                                   |  7 +++
>  drivers/misc/Kconfig                          |  9 +++
>  drivers/misc/Makefile                         |  1 +
>  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
>  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
>  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
>  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
>  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
>  9 files changed, 228 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
>  create mode 100644 drivers/misc/platform-lockdown-attrs.c
>  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> 
> diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> new file mode 100644
> index 000000000000..6034d6cbefac
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> @@ -0,0 +1,23 @@
> +What:		/sys/class/platform-lockdown/bioswe
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set BIOS Write Enable.
> +		0: writes disabled, 1: writes enabled.
> +Users:		https://github.com/fwupd/fwupd
> +
> +What:		/sys/class/platform-lockdown/ble
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set Bios Lock Enable.

                                           BIOS

> +		0: SMM lock disabled, 1: SMM lock enabled.
> +Users:		https://github.com/fwupd/fwupd
> +
> +What:		/sys/class/platform-lockdown/smm_bwp
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set SMM Bios Write Protect.

                                               BIOS

> +		0: writes disabled unless in SMM, 1: writes enabled.
> +Users:		https://github.com/fwupd/fwupd



cheers.
-- 
~Randy

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Thu, Jul 30, 2020 at 7:33 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi,
>
> Could we get some consistency in the use of "bios" vs. "Bios" vs. "BIOS", please.
> BIOS is preferred IMO.

OK for the next patch.

>
> On 7/30/20 2:41 PM, Daniel Gutson wrote:
> >
> > This initial version exports the BIOS Write Enable (bioswe),
> > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> > fields of the Bios Control register. 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
>
>                                       available
>
> > is turned on.
> >
> > The patch provides a new misc driver, as proposed in the previous patch,
> > that provides a registration function for HW Driver devices to register
> > class_attributes.
> > In this case, the intel SPI flash chip (intel-spi) registers three
> > class_attributes corresponding to the fields mentioned above.
> >
> > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> > ---
> >  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
> >  MAINTAINERS                                   |  7 +++
> >  drivers/misc/Kconfig                          |  9 +++
> >  drivers/misc/Makefile                         |  1 +
> >  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
> >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
> >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
> >  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
> >  9 files changed, 228 insertions(+)
> >  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
> >  create mode 100644 drivers/misc/platform-lockdown-attrs.c
> >  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> >
> > diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > new file mode 100644
> > index 000000000000..6034d6cbefac
> > --- /dev/null
> > +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > @@ -0,0 +1,23 @@
> > +What:                /sys/class/platform-lockdown/bioswe
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set BIOS Write Enable.
> > +             0: writes disabled, 1: writes enabled.
> > +Users:               https://github.com/fwupd/fwupd
> > +
> > +What:                /sys/class/platform-lockdown/ble
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set Bios Lock Enable.
>
>                                            BIOS
>
> > +             0: SMM lock disabled, 1: SMM lock enabled.
> > +Users:               https://github.com/fwupd/fwupd
> > +
> > +What:                /sys/class/platform-lockdown/smm_bwp
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set SMM Bios Write Protect.
>
>                                                BIOS
>
> > +             0: writes disabled unless in SMM, 1: writes enabled.
> > +Users:               https://github.com/fwupd/fwupd
>
>
>
> cheers.
> --
> ~Randy
>


-- 
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Thu, Jul 30, 2020 at 06:41:36PM -0300, Daniel Gutson wrote:
> This patch exports information about the platform lockdown
> firmware configuration in the sysfs filesystem.
> In this initial patch, I include some configuration attributes
> for the system SPI chip.
> 
> This initial version exports the BIOS Write Enable (bioswe),
> BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> fields of the Bios Control register. 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.
> 
> The patch provides a new misc driver, as proposed in the previous patch,
> that provides a registration function for HW Driver devices to register
> class_attributes.
> In this case, the intel SPI flash chip (intel-spi) registers three
> class_attributes corresponding to the fields mentioned above.

Better, but you are abusing sysfs (note, no CAPS) a lot here...


> 
> Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> ---
>  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
>  MAINTAINERS                                   |  7 +++
>  drivers/misc/Kconfig                          |  9 +++
>  drivers/misc/Makefile                         |  1 +
>  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
>  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
>  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
>  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
>  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
>  9 files changed, 228 insertions(+)
>  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
>  create mode 100644 drivers/misc/platform-lockdown-attrs.c
>  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> 
> diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> new file mode 100644
> index 000000000000..6034d6cbefac
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> @@ -0,0 +1,23 @@
> +What:		/sys/class/platform-lockdown/bioswe
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set BIOS Write Enable.
> +		0: writes disabled, 1: writes enabled.
> +Users:		https://github.com/fwupd/fwupd
> +
> +What:		/sys/class/platform-lockdown/ble
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set Bios Lock Enable.
> +		0: SMM lock disabled, 1: SMM lock enabled.
> +Users:		https://github.com/fwupd/fwupd
> +
> +What:		/sys/class/platform-lockdown/smm_bwp
> +Date:		July 2020
> +KernelVersion:	5.8.0
> +Contact:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +Description:	If the system firmware set SMM Bios Write Protect.
> +		0: writes disabled unless in SMM, 1: writes enabled.
> +Users:		https://github.com/fwupd/fwupd
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f0569cf304ca..771ed1693d28 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13608,6 +13608,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
>  F:	drivers/iio/chemical/pms7003.c
>  
> +PLATFORM LOCKDOWN ATTRIBUTES MODULE
> +M:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +S:	Supported
> +F:	Documentation/ABI/sysfs-class-platform-lockdown
> +F:	drivers/misc/platform-lockdown-attrs.c
> +F:	include/linux/platform_data/platform-lockdown-attrs.h
> +
>  PLX DMA DRIVER
>  M:	Logan Gunthorpe <logang@deltatee.com>
>  S:	Maintained
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index e1b1ba5e2b92..058d4ba3cefd 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -456,6 +456,15 @@ config PVPANIC
>  	  a paravirtualized device provided by QEMU; it lets a virtual machine
>  	  (guest) communicate panic events to the host.
>  
> +config PLATFORM_LOCKDOWN_ATTRS
> +	tristate "Platform lockdown information in the SYSFS"

"Platform lockdown information for some hardware information displayed
in sysfs" ?

> +	depends on SYSFS
> +	help
> +	  This kernel module is a helper driver to provide information about
> +	  platform lockdown settings and configuration.

Is that what this really is?

> +	  This module is used by other device drivers -such as the intel-spi-
> +	  to publish the information in /sys/class/platform-lockdown.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index c7bd01ac6291..e29b45c564f9 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
>  obj-$(CONFIG_HABANA_AI)		+= habanalabs/
>  obj-$(CONFIG_UACCE)		+= uacce/
>  obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
> +obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS)	+= platform-lockdown-attrs.o
> diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c
> new file mode 100644
> index 000000000000..d08b3d895064
> --- /dev/null
> +++ b/drivers/misc/platform-lockdown-attrs.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Platform lockdown attributes kernel module
> + *
> + * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
> + * Copyright (C) 2020 Eclypsium Inc.
> + */
> +#include <linux/kobject.h>
> +#include <linux/sysfs.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/platform_data/platform-lockdown-attrs.h>
> +
> +static struct class platform_lockdown_class = {
> +	.name	= "platform-lockdown",
> +	.owner	= THIS_MODULE,
> +};
> +
> +int register_platform_lockdown_attribute(
> +	struct class_attribute *lockdown_attribute)
> +{
> +	/* attempt to create the file: */
> +	return class_create_file(&platform_lockdown_class,
> +				   lockdown_attribute);
> +}
> +EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute);

nit, global symbols should put the noun first:
	platform_lockdown_attribute_register()

But, this all is not ok.

You are trying to create/remove random sysfs files that end up in a
single class.  What you want to do is create a device that is associated
with this class, and make that device a child of the device you are
registering these attributes for.

Think of this as an input device.  You don't put the random input
attributes all in one place, you create a new device that represents the
input interface and register that.  Then userspace can iterate over all
devices that are of the input class and see the individual attributes of
them.

Make more sense?

So you need something like:
	platform_lockdown_device_create()

that returns a 'struct device' that is associated with the new class
device.

thanks,

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Fri, Jul 31, 2020 at 4:01 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 30, 2020 at 06:41:36PM -0300, Daniel Gutson wrote:
> > This patch exports information about the platform lockdown
> > firmware configuration in the sysfs filesystem.
> > In this initial patch, I include some configuration attributes
> > for the system SPI chip.
> >
> > This initial version exports the BIOS Write Enable (bioswe),
> > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> > fields of the Bios Control register. 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.
> >
> > The patch provides a new misc driver, as proposed in the previous patch,
> > that provides a registration function for HW Driver devices to register
> > class_attributes.
> > In this case, the intel SPI flash chip (intel-spi) registers three
> > class_attributes corresponding to the fields mentioned above.
>
> Better, but you are abusing sysfs (note, no CAPS) a lot here...
>
>
> >
> > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> > ---
> >  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
> >  MAINTAINERS                                   |  7 +++
> >  drivers/misc/Kconfig                          |  9 +++
> >  drivers/misc/Makefile                         |  1 +
> >  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
> >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
> >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
> >  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
> >  9 files changed, 228 insertions(+)
> >  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
> >  create mode 100644 drivers/misc/platform-lockdown-attrs.c
> >  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> >
> > diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > new file mode 100644
> > index 000000000000..6034d6cbefac
> > --- /dev/null
> > +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > @@ -0,0 +1,23 @@
> > +What:                /sys/class/platform-lockdown/bioswe
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set BIOS Write Enable.
> > +             0: writes disabled, 1: writes enabled.
> > +Users:               https://github.com/fwupd/fwupd
> > +
> > +What:                /sys/class/platform-lockdown/ble
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set Bios Lock Enable.
> > +             0: SMM lock disabled, 1: SMM lock enabled.
> > +Users:               https://github.com/fwupd/fwupd
> > +
> > +What:                /sys/class/platform-lockdown/smm_bwp
> > +Date:                July 2020
> > +KernelVersion:       5.8.0
> > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > +Description: If the system firmware set SMM Bios Write Protect.
> > +             0: writes disabled unless in SMM, 1: writes enabled.
> > +Users:               https://github.com/fwupd/fwupd
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f0569cf304ca..771ed1693d28 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13608,6 +13608,13 @@ S:   Maintained
> >  F:   Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
> >  F:   drivers/iio/chemical/pms7003.c
> >
> > +PLATFORM LOCKDOWN ATTRIBUTES MODULE
> > +M:   Daniel Gutson <daniel.gutson@eclypsium.com>
> > +S:   Supported
> > +F:   Documentation/ABI/sysfs-class-platform-lockdown
> > +F:   drivers/misc/platform-lockdown-attrs.c
> > +F:   include/linux/platform_data/platform-lockdown-attrs.h
> > +
> >  PLX DMA DRIVER
> >  M:   Logan Gunthorpe <logang@deltatee.com>
> >  S:   Maintained
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index e1b1ba5e2b92..058d4ba3cefd 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -456,6 +456,15 @@ config PVPANIC
> >         a paravirtualized device provided by QEMU; it lets a virtual machine
> >         (guest) communicate panic events to the host.
> >
> > +config PLATFORM_LOCKDOWN_ATTRS
> > +     tristate "Platform lockdown information in the SYSFS"
>
> "Platform lockdown information for some hardware information displayed
> in sysfs" ?
>
> > +     depends on SYSFS
> > +     help
> > +       This kernel module is a helper driver to provide information about
> > +       platform lockdown settings and configuration.
>
> Is that what this really is?
>
> > +       This module is used by other device drivers -such as the intel-spi-
> > +       to publish the information in /sys/class/platform-lockdown.
> > +
> >  source "drivers/misc/c2port/Kconfig"
> >  source "drivers/misc/eeprom/Kconfig"
> >  source "drivers/misc/cb710/Kconfig"
> > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > index c7bd01ac6291..e29b45c564f9 100644
> > --- a/drivers/misc/Makefile
> > +++ b/drivers/misc/Makefile
> > @@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)       += pvpanic.o
> >  obj-$(CONFIG_HABANA_AI)              += habanalabs/
> >  obj-$(CONFIG_UACCE)          += uacce/
> >  obj-$(CONFIG_XILINX_SDFEC)   += xilinx_sdfec.o
> > +obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS)        += platform-lockdown-attrs.o
> > diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c
> > new file mode 100644
> > index 000000000000..d08b3d895064
> > --- /dev/null
> > +++ b/drivers/misc/platform-lockdown-attrs.c
> > @@ -0,0 +1,57 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Platform lockdown attributes kernel module
> > + *
> > + * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
> > + * Copyright (C) 2020 Eclypsium Inc.
> > + */
> > +#include <linux/kobject.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_data/platform-lockdown-attrs.h>
> > +
> > +static struct class platform_lockdown_class = {
> > +     .name   = "platform-lockdown",
> > +     .owner  = THIS_MODULE,
> > +};
> > +
> > +int register_platform_lockdown_attribute(
> > +     struct class_attribute *lockdown_attribute)
> > +{
> > +     /* attempt to create the file: */
> > +     return class_create_file(&platform_lockdown_class,
> > +                                lockdown_attribute);
> > +}
> > +EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute);
>
> nit, global symbols should put the noun first:
>         platform_lockdown_attribute_register()
>
> But, this all is not ok.
>
> You are trying to create/remove random sysfs files that end up in a
> single class.  What you want to do is create a device that is associated
> with this class, and make that device a child of the device you are
> registering these attributes for.
>
> Think of this as an input device.  You don't put the random input
> attributes all in one place, you create a new device that represents the
> input interface and register that.  Then userspace can iterate over all
> devices that are of the input class and see the individual attributes of
> them.

So I create the child device.
And these attributes, will be class attributes or device attributes of
the child device?

>
> Make more sense?
>
> So you need something like:
>         platform_lockdown_device_create()
>
> that returns a 'struct device' that is associated with the new class
> device.
>
> thanks,
>
> greg k-h



-- 
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Fri, Jul 31, 2020 at 10:30:43AM -0300, Daniel Gutson wrote:
> On Fri, Jul 31, 2020 at 4:01 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jul 30, 2020 at 06:41:36PM -0300, Daniel Gutson wrote:
> > > This patch exports information about the platform lockdown
> > > firmware configuration in the sysfs filesystem.
> > > In this initial patch, I include some configuration attributes
> > > for the system SPI chip.
> > >
> > > This initial version exports the BIOS Write Enable (bioswe),
> > > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> > > fields of the Bios Control register. 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.
> > >
> > > The patch provides a new misc driver, as proposed in the previous patch,
> > > that provides a registration function for HW Driver devices to register
> > > class_attributes.
> > > In this case, the intel SPI flash chip (intel-spi) registers three
> > > class_attributes corresponding to the fields mentioned above.
> >
> > Better, but you are abusing sysfs (note, no CAPS) a lot here...
> >
> >
> > >
> > > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> > > ---
> > >  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
> > >  MAINTAINERS                                   |  7 +++
> > >  drivers/misc/Kconfig                          |  9 +++
> > >  drivers/misc/Makefile                         |  1 +
> > >  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
> > >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> > >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
> > >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
> > >  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
> > >  9 files changed, 228 insertions(+)
> > >  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
> > >  create mode 100644 drivers/misc/platform-lockdown-attrs.c
> > >  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> > >
> > > diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > > new file mode 100644
> > > index 000000000000..6034d6cbefac
> > > --- /dev/null
> > > +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > > @@ -0,0 +1,23 @@
> > > +What:                /sys/class/platform-lockdown/bioswe
> > > +Date:                July 2020
> > > +KernelVersion:       5.8.0
> > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > +Description: If the system firmware set BIOS Write Enable.
> > > +             0: writes disabled, 1: writes enabled.
> > > +Users:               https://github.com/fwupd/fwupd
> > > +
> > > +What:                /sys/class/platform-lockdown/ble
> > > +Date:                July 2020
> > > +KernelVersion:       5.8.0
> > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > +Description: If the system firmware set Bios Lock Enable.
> > > +             0: SMM lock disabled, 1: SMM lock enabled.
> > > +Users:               https://github.com/fwupd/fwupd
> > > +
> > > +What:                /sys/class/platform-lockdown/smm_bwp
> > > +Date:                July 2020
> > > +KernelVersion:       5.8.0
> > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > +Description: If the system firmware set SMM Bios Write Protect.
> > > +             0: writes disabled unless in SMM, 1: writes enabled.
> > > +Users:               https://github.com/fwupd/fwupd
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index f0569cf304ca..771ed1693d28 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -13608,6 +13608,13 @@ S:   Maintained
> > >  F:   Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
> > >  F:   drivers/iio/chemical/pms7003.c
> > >
> > > +PLATFORM LOCKDOWN ATTRIBUTES MODULE
> > > +M:   Daniel Gutson <daniel.gutson@eclypsium.com>
> > > +S:   Supported
> > > +F:   Documentation/ABI/sysfs-class-platform-lockdown
> > > +F:   drivers/misc/platform-lockdown-attrs.c
> > > +F:   include/linux/platform_data/platform-lockdown-attrs.h
> > > +
> > >  PLX DMA DRIVER
> > >  M:   Logan Gunthorpe <logang@deltatee.com>
> > >  S:   Maintained
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > index e1b1ba5e2b92..058d4ba3cefd 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -456,6 +456,15 @@ config PVPANIC
> > >         a paravirtualized device provided by QEMU; it lets a virtual machine
> > >         (guest) communicate panic events to the host.
> > >
> > > +config PLATFORM_LOCKDOWN_ATTRS
> > > +     tristate "Platform lockdown information in the SYSFS"
> >
> > "Platform lockdown information for some hardware information displayed
> > in sysfs" ?
> >
> > > +     depends on SYSFS
> > > +     help
> > > +       This kernel module is a helper driver to provide information about
> > > +       platform lockdown settings and configuration.
> >
> > Is that what this really is?
> >
> > > +       This module is used by other device drivers -such as the intel-spi-
> > > +       to publish the information in /sys/class/platform-lockdown.
> > > +
> > >  source "drivers/misc/c2port/Kconfig"
> > >  source "drivers/misc/eeprom/Kconfig"
> > >  source "drivers/misc/cb710/Kconfig"
> > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > index c7bd01ac6291..e29b45c564f9 100644
> > > --- a/drivers/misc/Makefile
> > > +++ b/drivers/misc/Makefile
> > > @@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)       += pvpanic.o
> > >  obj-$(CONFIG_HABANA_AI)              += habanalabs/
> > >  obj-$(CONFIG_UACCE)          += uacce/
> > >  obj-$(CONFIG_XILINX_SDFEC)   += xilinx_sdfec.o
> > > +obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS)        += platform-lockdown-attrs.o
> > > diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c
> > > new file mode 100644
> > > index 000000000000..d08b3d895064
> > > --- /dev/null
> > > +++ b/drivers/misc/platform-lockdown-attrs.c
> > > @@ -0,0 +1,57 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Platform lockdown attributes kernel module
> > > + *
> > > + * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
> > > + * Copyright (C) 2020 Eclypsium Inc.
> > > + */
> > > +#include <linux/kobject.h>
> > > +#include <linux/sysfs.h>
> > > +#include <linux/module.h>
> > > +#include <linux/init.h>
> > > +#include <linux/list.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/platform_data/platform-lockdown-attrs.h>
> > > +
> > > +static struct class platform_lockdown_class = {
> > > +     .name   = "platform-lockdown",
> > > +     .owner  = THIS_MODULE,
> > > +};
> > > +
> > > +int register_platform_lockdown_attribute(
> > > +     struct class_attribute *lockdown_attribute)
> > > +{
> > > +     /* attempt to create the file: */
> > > +     return class_create_file(&platform_lockdown_class,
> > > +                                lockdown_attribute);
> > > +}
> > > +EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute);
> >
> > nit, global symbols should put the noun first:
> >         platform_lockdown_attribute_register()
> >
> > But, this all is not ok.
> >
> > You are trying to create/remove random sysfs files that end up in a
> > single class.  What you want to do is create a device that is associated
> > with this class, and make that device a child of the device you are
> > registering these attributes for.
> >
> > Think of this as an input device.  You don't put the random input
> > attributes all in one place, you create a new device that represents the
> > input interface and register that.  Then userspace can iterate over all
> > devices that are of the input class and see the individual attributes of
> > them.
> 
> So I create the child device.
> And these attributes, will be class attributes or device attributes of
> the child device?

Device attributes of course :)

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Fri, Jul 31, 2020 at 11:15 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jul 31, 2020 at 10:30:43AM -0300, Daniel Gutson wrote:
> > On Fri, Jul 31, 2020 at 4:01 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Jul 30, 2020 at 06:41:36PM -0300, Daniel Gutson wrote:
> > > > This patch exports information about the platform lockdown
> > > > firmware configuration in the sysfs filesystem.
> > > > In this initial patch, I include some configuration attributes
> > > > for the system SPI chip.
> > > >
> > > > This initial version exports the BIOS Write Enable (bioswe),
> > > > BIOS Lock Enable (ble), and the SMM Bios Write Protect (SMM_BWP)
> > > > fields of the Bios Control register. 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.
> > > >
> > > > The patch provides a new misc driver, as proposed in the previous patch,
> > > > that provides a registration function for HW Driver devices to register
> > > > class_attributes.
> > > > In this case, the intel SPI flash chip (intel-spi) registers three
> > > > class_attributes corresponding to the fields mentioned above.
> > >
> > > Better, but you are abusing sysfs (note, no CAPS) a lot here...
> > >
> > >
> > > >
> > > > Signed-off-by: Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > ---
> > > >  .../ABI/stable/sysfs-class-platform-lockdown  | 23 +++++++
> > > >  MAINTAINERS                                   |  7 +++
> > > >  drivers/misc/Kconfig                          |  9 +++
> > > >  drivers/misc/Makefile                         |  1 +
> > > >  drivers/misc/platform-lockdown-attrs.c        | 57 +++++++++++++++++
> > > >  drivers/mtd/spi-nor/controllers/Kconfig       |  1 +
> > > >  .../mtd/spi-nor/controllers/intel-spi-pci.c   | 49 +++++++++++++++
> > > >  drivers/mtd/spi-nor/controllers/intel-spi.c   | 62 +++++++++++++++++++
> > > >  .../platform_data/platform-lockdown-attrs.h   | 19 ++++++
> > > >  9 files changed, 228 insertions(+)
> > > >  create mode 100644 Documentation/ABI/stable/sysfs-class-platform-lockdown
> > > >  create mode 100644 drivers/misc/platform-lockdown-attrs.c
> > > >  create mode 100644 include/linux/platform_data/platform-lockdown-attrs.h
> > > >
> > > > diff --git a/Documentation/ABI/stable/sysfs-class-platform-lockdown b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > > > new file mode 100644
> > > > index 000000000000..6034d6cbefac
> > > > --- /dev/null
> > > > +++ b/Documentation/ABI/stable/sysfs-class-platform-lockdown
> > > > @@ -0,0 +1,23 @@
> > > > +What:                /sys/class/platform-lockdown/bioswe
> > > > +Date:                July 2020
> > > > +KernelVersion:       5.8.0
> > > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > +Description: If the system firmware set BIOS Write Enable.
> > > > +             0: writes disabled, 1: writes enabled.
> > > > +Users:               https://github.com/fwupd/fwupd
> > > > +
> > > > +What:                /sys/class/platform-lockdown/ble
> > > > +Date:                July 2020
> > > > +KernelVersion:       5.8.0
> > > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > +Description: If the system firmware set Bios Lock Enable.
> > > > +             0: SMM lock disabled, 1: SMM lock enabled.
> > > > +Users:               https://github.com/fwupd/fwupd
> > > > +
> > > > +What:                /sys/class/platform-lockdown/smm_bwp
> > > > +Date:                July 2020
> > > > +KernelVersion:       5.8.0
> > > > +Contact:     Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > +Description: If the system firmware set SMM Bios Write Protect.
> > > > +             0: writes disabled unless in SMM, 1: writes enabled.
> > > > +Users:               https://github.com/fwupd/fwupd
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index f0569cf304ca..771ed1693d28 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -13608,6 +13608,13 @@ S:   Maintained
> > > >  F:   Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.yaml
> > > >  F:   drivers/iio/chemical/pms7003.c
> > > >
> > > > +PLATFORM LOCKDOWN ATTRIBUTES MODULE
> > > > +M:   Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > +S:   Supported
> > > > +F:   Documentation/ABI/sysfs-class-platform-lockdown
> > > > +F:   drivers/misc/platform-lockdown-attrs.c
> > > > +F:   include/linux/platform_data/platform-lockdown-attrs.h
> > > > +
> > > >  PLX DMA DRIVER
> > > >  M:   Logan Gunthorpe <logang@deltatee.com>
> > > >  S:   Maintained
> > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > > index e1b1ba5e2b92..058d4ba3cefd 100644
> > > > --- a/drivers/misc/Kconfig
> > > > +++ b/drivers/misc/Kconfig
> > > > @@ -456,6 +456,15 @@ config PVPANIC
> > > >         a paravirtualized device provided by QEMU; it lets a virtual machine
> > > >         (guest) communicate panic events to the host.
> > > >
> > > > +config PLATFORM_LOCKDOWN_ATTRS
> > > > +     tristate "Platform lockdown information in the SYSFS"
> > >
> > > "Platform lockdown information for some hardware information displayed
> > > in sysfs" ?
> > >
> > > > +     depends on SYSFS
> > > > +     help
> > > > +       This kernel module is a helper driver to provide information about
> > > > +       platform lockdown settings and configuration.
> > >
> > > Is that what this really is?
> > >
> > > > +       This module is used by other device drivers -such as the intel-spi-
> > > > +       to publish the information in /sys/class/platform-lockdown.
> > > > +
> > > >  source "drivers/misc/c2port/Kconfig"
> > > >  source "drivers/misc/eeprom/Kconfig"
> > > >  source "drivers/misc/cb710/Kconfig"
> > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > > index c7bd01ac6291..e29b45c564f9 100644
> > > > --- a/drivers/misc/Makefile
> > > > +++ b/drivers/misc/Makefile
> > > > @@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)       += pvpanic.o
> > > >  obj-$(CONFIG_HABANA_AI)              += habanalabs/
> > > >  obj-$(CONFIG_UACCE)          += uacce/
> > > >  obj-$(CONFIG_XILINX_SDFEC)   += xilinx_sdfec.o
> > > > +obj-$(CONFIG_PLATFORM_LOCKDOWN_ATTRS)        += platform-lockdown-attrs.o
> > > > diff --git a/drivers/misc/platform-lockdown-attrs.c b/drivers/misc/platform-lockdown-attrs.c
> > > > new file mode 100644
> > > > index 000000000000..d08b3d895064
> > > > --- /dev/null
> > > > +++ b/drivers/misc/platform-lockdown-attrs.c
> > > > @@ -0,0 +1,57 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Platform lockdown attributes kernel module
> > > > + *
> > > > + * Copyright (C) 2020 Daniel Gutson <daniel.gutson@eclypsium.com>
> > > > + * Copyright (C) 2020 Eclypsium Inc.
> > > > + */
> > > > +#include <linux/kobject.h>
> > > > +#include <linux/sysfs.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/list.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/platform_data/platform-lockdown-attrs.h>
> > > > +
> > > > +static struct class platform_lockdown_class = {
> > > > +     .name   = "platform-lockdown",
> > > > +     .owner  = THIS_MODULE,
> > > > +};
> > > > +
> > > > +int register_platform_lockdown_attribute(
> > > > +     struct class_attribute *lockdown_attribute)
> > > > +{
> > > > +     /* attempt to create the file: */
> > > > +     return class_create_file(&platform_lockdown_class,
> > > > +                                lockdown_attribute);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(register_platform_lockdown_attribute);
> > >
> > > nit, global symbols should put the noun first:
> > >         platform_lockdown_attribute_register()
> > >
> > > But, this all is not ok.
> > >
> > > You are trying to create/remove random sysfs files that end up in a
> > > single class.  What you want to do is create a device that is associated
> > > with this class, and make that device a child of the device you are
> > > registering these attributes for.
> > >
> > > Think of this as an input device.  You don't put the random input
> > > attributes all in one place, you create a new device that represents the
> > > input interface and register that.

I'm having trouble with this. What's the dev_t for the child devices?
I'm doing
    child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
NULL, "child");
pdev is the pci_device (intel-spi-pci)
dmesg shows

    sysfs: cannot create duplicate filename '/class/my-class'
    (call trace)
    kobject_add_internal failed for my-class with -EEXIST, don't try
to register things with the same name in the same directory.

Even worse when I tried to remove the class:
I can't use device_destroy because there is no dev_t, so since it is
    put_device(child_device);
    device_unregister(child_device);
that caused a core dump too.
I tried also doing nothing with the child_device, but then I get a core dump.

Is all these caused by the MKDEV(0, 0) ?
.

  Then userspace can iterate over all
> > > devices that are of the input class and see the individual attributes of
> > > them.
> >
> > So I create the child device.
> > And these attributes, will be class attributes or device attributes of
> > the child device?
>
> Device attributes of course :)
>
> greg k-h
--
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > Think of this as an input device.  You don't put the random input
> > > > attributes all in one place, you create a new device that represents the
> > > > input interface and register that.
> 
> I'm having trouble with this. What's the dev_t for the child devices?
> I'm doing
>     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> NULL, "child");
> pdev is the pci_device (intel-spi-pci)
> dmesg shows
> 
>     sysfs: cannot create duplicate filename '/class/my-class'
>     (call trace)
>     kobject_add_internal failed for my-class with -EEXIST, don't try
> to register things with the same name in the same directory.

Without seeing all of your code, I can't tell you what you are doing
wrong, but the kernel should be giving you a huge hint here...

Don't create duplicate names in the same subdirectory.

thanks,

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > Think of this as an input device.  You don't put the random input
> > > > > attributes all in one place, you create a new device that represents the
> > > > > input interface and register that.
> >
> > I'm having trouble with this. What's the dev_t for the child devices?
> > I'm doing
> >     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > NULL, "child");
> > pdev is the pci_device (intel-spi-pci)
> > dmesg shows
> >
> >     sysfs: cannot create duplicate filename '/class/my-class'
> >     (call trace)
> >     kobject_add_internal failed for my-class with -EEXIST, don't try
> > to register things with the same name in the same directory.
>
> Without seeing all of your code, I can't tell you what you are doing
> wrong, but the kernel should be giving you a huge hint here...
>
> Don't create duplicate names in the same subdirectory.

I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
create_device, which I inferred so from the documentation.
Here is the listing

static ssize_t howareyou_show(struct class *class, struct class_attribute *attr,
    char *buf)
{
return sprintf(buf, "%s\n", "How are you?");
}
static CLASS_ATTR_RO(howareyou);

static struct class my_class = {
        .name =         "my-class",
        .owner =        THIS_MODULE,
};

struct device* child_device;


static int mypci_probe(struct pci_dev *pdev,
       const struct pci_device_id *id)
{
int ret;

ret = pcim_enable_device(pdev);
if (ret)
return ret;

ret = class_register(&my_class);
if (ret < 0)
return ret;


pr_info("DFG: Recognized. DID: %lx\n", (unsigned long int)id->driver_data);
pr_info("DFG: device DID: %lx\n", (unsigned long int)pdev->device);

ret = class_create_file(&my_class, &class_attr_howareyou);
if (ret != 0)
{
    pr_err("DFG class create file error: %d\n", ret);
        class_unregister(&my_class);
        return ret;
}

    child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
NULL, "child");
if (child_device == NULL)
{
    pr_err("DFG error child device NULL");
}

return ret;
}

static void mypci_remove(struct pci_dev *pdev)
{
/* I tried enabling and disabling this code
if (child_device != NULL)
{
put_device(child_device);
device_unregister(child_device);
}
*/

    class_remove_file(&my_class, &class_attr_howareyou);
    class_unregister(&my_class);
}

static const struct pci_device_id my_dids[] = {
{ PCI_VDEVICE(INTEL, 0xa30e), (unsigned long)0xa30e },
{ PCI_VDEVICE(INTEL, 0xa324), (unsigned long)0xa324 },
{ },
};
MODULE_DEVICE_TABLE(pci, my_dids);

static struct pci_driver my_pci_driver = {
.name = "dfg-pci",
.id_table = my_dids,
.probe = mypci_probe,
.remove = mypci_remove,
};

module_pci_driver(my_pci_driver);

MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Daniel Gutson <daniel.gutson@eclypsium.com>");


>
> thanks,
>
> greg k-h



-- 
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Tue, Aug 04, 2020 at 10:50:13AM -0300, Daniel Gutson wrote:
> On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > > Think of this as an input device.  You don't put the random input
> > > > > > attributes all in one place, you create a new device that represents the
> > > > > > input interface and register that.
> > >
> > > I'm having trouble with this. What's the dev_t for the child devices?
> > > I'm doing
> > >     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > NULL, "child");
> > > pdev is the pci_device (intel-spi-pci)
> > > dmesg shows
> > >
> > >     sysfs: cannot create duplicate filename '/class/my-class'
> > >     (call trace)
> > >     kobject_add_internal failed for my-class with -EEXIST, don't try
> > > to register things with the same name in the same directory.
> >
> > Without seeing all of your code, I can't tell you what you are doing
> > wrong, but the kernel should be giving you a huge hint here...
> >
> > Don't create duplicate names in the same subdirectory.
> 
> I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
> create_device, which I inferred so from the documentation.

Yes it is, but that's not the error given to you :)

Many in-kernel users call device_create() with MKDEV(0, 0)

> Here is the listing

<snip>

It's not in any format to read, please never strip leading whitespace,
it hurts my brain...

thanks,

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> On Tue, Aug 4, 2020 at 11:23 AM Greg Kroah-Hartman <
> gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Aug 04, 2020 at 10:50:13AM -0300, Daniel Gutson wrote:
> > > On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > > > > Think of this as an input device.  You don't put the random
> input
> > > > > > > > attributes all in one place, you create a new device that
> represents the
> > > > > > > > input interface and register that.
> > > > >
> > > > > I'm having trouble with this. What's the dev_t for the child
> devices?
> > > > > I'm doing
> > > > >     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > > > NULL, "child");
> > > > > pdev is the pci_device (intel-spi-pci)
> > > > > dmesg shows
> > > > >
> > > > >     sysfs: cannot create duplicate filename '/class/my-class'
> > > > >     (call trace)
> > > > >     kobject_add_internal failed for my-class with -EEXIST, don't try
> > > > > to register things with the same name in the same directory.
> > > >
> > > > Without seeing all of your code, I can't tell you what you are doing
> > > > wrong, but the kernel should be giving you a huge hint here...
> > > >
> > > > Don't create duplicate names in the same subdirectory.
> > >
> > > I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
> > > create_device, which I inferred so from the documentation.
> >
> > Yes it is, but that's not the error given to you :)
> >
> > Many in-kernel users call device_create() with MKDEV(0, 0)
> >
> > > Here is the listing
> >
> > <snip>
> >
> > It's not in any format to read, please never strip leading whitespace,
> > it hurts my brain...
> 
> (trying again)
> Also, this is in pastebin:  https://pastebin.com/8Ye9eUm5
> 
> #include <linux/kobject.h>
> #include <linux/sysfs.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/list.h>
> #include <linux/slab.h>
> #include <linux/device.h>
> #include <linux/pci.h>
> 
> static ssize_t howareyou_show(struct class *class, struct class_attribute
> *attr,
>    char *buf)
> {
>         return sprintf(buf, "%s\n", "How are you?");
> }
> static CLASS_ATTR_RO(howareyou);

These are rare, as they are "global" for a class, are you sure you want
that?

> 
> static struct class my_class = {
>         .name =         "my-class",
>         .owner =        THIS_MODULE,
> };
> 
> struct device* child_device;
> 
> static int mypci_probe(struct pci_dev *pdev,
>       const struct pci_device_id *id)
> {
>         int ret;
> 
>         ret = pcim_enable_device(pdev);
>         if (ret)
>                 return ret;
> 
>         ret = class_register(&my_class);
>         if (ret < 0)
>                 return ret;
> 
> 
>         pr_info("DFG: Recognized. DID: %lx\n", (unsigned long
> int)id->driver_data);
>         pr_info("DFG: device DID: %lx\n", (unsigned long int)pdev->device);
> 
>         ret = class_create_file(&my_class, &class_attr_howareyou);
>         if (ret != 0) {
>                pr_err("DFG class create file error: %d\n", ret);
>                class_unregister(&my_class);
>                return ret;
>        }
> 
>        child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> NULL, "child");
>         if (child_device == NULL) {
>                pr_err("DFG error child device NULL");
>         }
> 
>         return ret;
> }
> 


Looks sane, what does your kernel log say when you load this?

thanks,

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Tue, Aug 4, 2020 at 11:43 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> > On Tue, Aug 4, 2020 at 11:23 AM Greg Kroah-Hartman <
> > gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 10:50:13AM -0300, Daniel Gutson wrote:
> > > > On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > > > > > Think of this as an input device.  You don't put the random
> > input
> > > > > > > > > attributes all in one place, you create a new device that
> > represents the
> > > > > > > > > input interface and register that.
> > > > > >
> > > > > > I'm having trouble with this. What's the dev_t for the child
> > devices?
> > > > > > I'm doing
> > > > > >     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > > > > NULL, "child");
> > > > > > pdev is the pci_device (intel-spi-pci)
> > > > > > dmesg shows
> > > > > >
> > > > > >     sysfs: cannot create duplicate filename '/class/my-class'
> > > > > >     (call trace)
> > > > > >     kobject_add_internal failed for my-class with -EEXIST, don't try
> > > > > > to register things with the same name in the same directory.
> > > > >
> > > > > Without seeing all of your code, I can't tell you what you are doing
> > > > > wrong, but the kernel should be giving you a huge hint here...
> > > > >
> > > > > Don't create duplicate names in the same subdirectory.
> > > >
> > > > I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
> > > > create_device, which I inferred so from the documentation.
> > >
> > > Yes it is, but that's not the error given to you :)
> > >
> > > Many in-kernel users call device_create() with MKDEV(0, 0)
> > >
> > > > Here is the listing
> > >
> > > <snip>
> > >
> > > It's not in any format to read, please never strip leading whitespace,
> > > it hurts my brain...
> >
> > (trying again)
> > Also, this is in pastebin:  https://pastebin.com/8Ye9eUm5
> >
> > #include <linux/kobject.h>
> > #include <linux/sysfs.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > #include <linux/list.h>
> > #include <linux/slab.h>
> > #include <linux/device.h>
> > #include <linux/pci.h>
> >
> > static ssize_t howareyou_show(struct class *class, struct class_attribute
> > *attr,
> >    char *buf)
> > {
> >         return sprintf(buf, "%s\n", "How are you?");
> > }
> > static CLASS_ATTR_RO(howareyou);
>
> These are rare, as they are "global" for a class, are you sure you want
> that?

I'm no longer using class attributes, this is from my previous
experiment. Sorry for the confusion.
In the "real" code I'll use DEVICE_ATTR_RO.

>
> >
> > static struct class my_class = {
> >         .name =         "my-class",
> >         .owner =        THIS_MODULE,
> > };
> >
> > struct device* child_device;
> >
> > static int mypci_probe(struct pci_dev *pdev,
> >       const struct pci_device_id *id)
> > {
> >         int ret;
> >
> >         ret = pcim_enable_device(pdev);
> >         if (ret)
> >                 return ret;
> >
> >         ret = class_register(&my_class);
> >         if (ret < 0)
> >                 return ret;
> >
> >
> >         pr_info("DFG: Recognized. DID: %lx\n", (unsigned long
> > int)id->driver_data);
> >         pr_info("DFG: device DID: %lx\n", (unsigned long int)pdev->device);
> >
> >         ret = class_create_file(&my_class, &class_attr_howareyou);
> >         if (ret != 0) {
> >                pr_err("DFG class create file error: %d\n", ret);
> >                class_unregister(&my_class);
> >                return ret;
> >        }
> >
> >        child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > NULL, "child");
> >         if (child_device == NULL) {
> >                pr_err("DFG error child device NULL");
> >         }
> >
> >         return ret;
> > }
> >
>
>
> Looks sane, what does your kernel log say when you load this?
first insmod, OK.
rmmod, OK.
Second insmod:

[ 4149.389133] sysfs: cannot create duplicate filename
'/devices/pci0000:00/0000:00:1f.0/my-class'
[ 4149.389135] CPU: 2 PID: 8900 Comm: insmod Tainted: G           OE
  5.8.0+ #1
[ 4149.389136] Hardware name: xxxxx
[ 4149.389136] Call Trace:
[ 4149.389141]  dump_stack+0x74/0x9a
[ 4149.389143]  sysfs_warn_dup.cold+0x17/0x35
[ 4149.389144]  sysfs_create_dir_ns+0xb8/0xd0
[ 4149.389146]  kobject_add_internal+0xbd/0x2b0
[ 4149.389147]  kobject_add+0x7e/0xb0
[ 4149.389149]  ? _cond_resched+0x19/0x30
[ 4149.389151]  ? kmem_cache_alloc_trace+0x16c/0x240
[ 4149.389154]  get_device_parent.isra.0+0x179/0x1b0
[ 4149.389155]  device_add+0xed/0x830
[ 4149.389156]  device_create_groups_vargs+0xd4/0xf0
[ 4149.389157]  device_create+0x49/0x60
[ 4149.389159]  mypci_probe.cold+0x79/0x9a [firmware_security_class]
[ 4149.389160]  local_pci_probe+0x48/0x80
[ 4149.389161]  pci_device_probe+0x10f/0x1c0
[ 4149.389162]  really_probe+0x159/0x3e0
[ 4149.389163]  driver_probe_device+0xe9/0x160
[ 4149.389164]  device_driver_attach+0x5d/0x70
[ 4149.389165]  __driver_attach+0x8f/0x150
[ 4149.389166]  ? device_driver_attach+0x70/0x70
[ 4149.389167]  bus_for_each_dev+0x7e/0xc0
[ 4149.389168]  driver_attach+0x1e/0x20
[ 4149.389168]  bus_add_driver+0x152/0x1f0
[ 4149.389169]  driver_register+0x74/0xd0
[ 4149.389170]  ? 0xffffffffc0cb8000
[ 4149.389171]  __pci_register_driver+0x54/0x60
[ 4149.389172]  my_pci_driver_init+0x23/0x1000 [firmware_security_class]
[ 4149.389174]  do_one_initcall+0x4a/0x200
[ 4149.389175]  ? _cond_resched+0x19/0x30
[ 4149.389176]  ? kmem_cache_alloc_trace+0x16c/0x240
[ 4149.389177]  do_init_module+0x62/0x240
[ 4149.389178]  load_module+0x291f/0x2b90
[ 4149.389180]  __do_sys_finit_module+0xbe/0x120
[ 4149.389181]  ? __do_sys_finit_module+0xbe/0x120
[ 4149.389182]  __x64_sys_finit_module+0x1a/0x20
[ 4149.389183]  do_syscall_64+0x52/0xc0
[ 4149.389184]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 4149.389185] RIP: 0033:0x7fbdc4b8c70d
[ 4149.389186] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e
fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 53 f7 0c 00 f7 d8 64 89
01 48
[ 4149.389187] RSP: 002b:00007ffd7f2b7d88 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 4149.389188] RAX: ffffffffffffffda RBX: 000055b301ef4770 RCX: 00007fbdc4b8c70d
[ 4149.389188] RDX: 0000000000000000 RSI: 000055b301b4d358 RDI: 0000000000000003
[ 4149.389188] RBP: 0000000000000000 R08: 0000000000000000 R09: 00007fbdc4c60260
[ 4149.389189] R10: 0000000000000003 R11: 0000000000000246 R12: 000055b301b4d358
[ 4149.389189] R13: 0000000000000000 R14: 000055b301ef7180 R15: 0000000000000000
[ 4149.389190] kobject_add_internal failed for my-class with -EEXIST,
don't try to register things with the same name in the same directory.



>
> thanks,
>
> greg k-h



-- 
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Tue, Aug 04, 2020 at 12:05:25PM -0300, Daniel Gutson wrote:
> On Tue, Aug 4, 2020 at 11:43 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> > > On Tue, Aug 4, 2020 at 11:23 AM Greg Kroah-Hartman <
> > > gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, Aug 04, 2020 at 10:50:13AM -0300, Daniel Gutson wrote:
> > > > > On Tue, Aug 4, 2020 at 3:41 AM Greg Kroah-Hartman
> > > > > <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Mon, Aug 03, 2020 at 07:04:56PM -0300, Daniel Gutson wrote:
> > > > > > > > > > Think of this as an input device.  You don't put the random
> > > input
> > > > > > > > > > attributes all in one place, you create a new device that
> > > represents the
> > > > > > > > > > input interface and register that.
> > > > > > >
> > > > > > > I'm having trouble with this. What's the dev_t for the child
> > > devices?
> > > > > > > I'm doing
> > > > > > >     child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > > > > > NULL, "child");
> > > > > > > pdev is the pci_device (intel-spi-pci)
> > > > > > > dmesg shows
> > > > > > >
> > > > > > >     sysfs: cannot create duplicate filename '/class/my-class'
> > > > > > >     (call trace)
> > > > > > >     kobject_add_internal failed for my-class with -EEXIST, don't try
> > > > > > > to register things with the same name in the same directory.
> > > > > >
> > > > > > Without seeing all of your code, I can't tell you what you are doing
> > > > > > wrong, but the kernel should be giving you a huge hint here...
> > > > > >
> > > > > > Don't create duplicate names in the same subdirectory.
> > > > >
> > > > > I'm not doing that. One of my questions is if MKDEV(0, 0) is valid for
> > > > > create_device, which I inferred so from the documentation.
> > > >
> > > > Yes it is, but that's not the error given to you :)
> > > >
> > > > Many in-kernel users call device_create() with MKDEV(0, 0)
> > > >
> > > > > Here is the listing
> > > >
> > > > <snip>
> > > >
> > > > It's not in any format to read, please never strip leading whitespace,
> > > > it hurts my brain...
> > >
> > > (trying again)
> > > Also, this is in pastebin:  https://pastebin.com/8Ye9eUm5
> > >
> > > #include <linux/kobject.h>
> > > #include <linux/sysfs.h>
> > > #include <linux/module.h>
> > > #include <linux/init.h>
> > > #include <linux/list.h>
> > > #include <linux/slab.h>
> > > #include <linux/device.h>
> > > #include <linux/pci.h>
> > >
> > > static ssize_t howareyou_show(struct class *class, struct class_attribute
> > > *attr,
> > >    char *buf)
> > > {
> > >         return sprintf(buf, "%s\n", "How are you?");
> > > }
> > > static CLASS_ATTR_RO(howareyou);
> >
> > These are rare, as they are "global" for a class, are you sure you want
> > that?
> 
> I'm no longer using class attributes, this is from my previous
> experiment. Sorry for the confusion.
> In the "real" code I'll use DEVICE_ATTR_RO.
> 
> >
> > >
> > > static struct class my_class = {
> > >         .name =         "my-class",
> > >         .owner =        THIS_MODULE,
> > > };
> > >
> > > struct device* child_device;
> > >
> > > static int mypci_probe(struct pci_dev *pdev,
> > >       const struct pci_device_id *id)
> > > {
> > >         int ret;
> > >
> > >         ret = pcim_enable_device(pdev);
> > >         if (ret)
> > >                 return ret;
> > >
> > >         ret = class_register(&my_class);
> > >         if (ret < 0)
> > >                 return ret;
> > >
> > >
> > >         pr_info("DFG: Recognized. DID: %lx\n", (unsigned long
> > > int)id->driver_data);
> > >         pr_info("DFG: device DID: %lx\n", (unsigned long int)pdev->device);
> > >
> > >         ret = class_create_file(&my_class, &class_attr_howareyou);
> > >         if (ret != 0) {
> > >                pr_err("DFG class create file error: %d\n", ret);
> > >                class_unregister(&my_class);
> > >                return ret;
> > >        }
> > >
> > >        child_device = device_create(&my_class, &pdev->dev, MKDEV(0, 0),
> > > NULL, "child");
> > >         if (child_device == NULL) {
> > >                pr_err("DFG error child device NULL");
> > >         }
> > >
> > >         return ret;
> > > }
> > >
> >
> >
> > Looks sane, what does your kernel log say when you load this?
> first insmod, OK.
> rmmod, OK.
> Second insmod:
> 
> [ 4149.389133] sysfs: cannot create duplicate filename
> '/devices/pci0000:00/0000:00:1f.0/my-class'

Ok, that means you did not clean up properly.

After rmmod see if you really did clean up all of the directories/files
you created.

The kernel is trying to be nice and give you a hint here :)

thanks,

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Greg Kroah-Hartman]

On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> static void mypci_remove(struct pci_dev *pdev)
> {
>         /*
>     I tried enabling and disabling this
>         if (child_device != NULL) {
>                 put_device(child_device);
>                 device_unregister(child_device);
>         }
>         */

You can just call device_destroy() here, but this should be the same.

But, if you have it commented out, that's not good, you have to clean
this up.

>         class_remove_file(&my_class, &class_attr_howareyou);

You don't always have to remove files you create, but it doesn't hurt.

>         class_unregister(&my_class);

class_destroy()?  But this is the same as well, so all is good.

Try running without the above code commented out.

greg k-h

Re: [PATCH] Platform lockdown information in SYSFS

[Daniel Gutson]

On Tue, Aug 4, 2020 at 1:42 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 04, 2020 at 11:37:02AM -0300, Daniel Gutson wrote:
> > static void mypci_remove(struct pci_dev *pdev)
> > {
> >         /*
> >     I tried enabling and disabling this
> >         if (child_device != NULL) {
> >                 put_device(child_device);
> >                 device_unregister(child_device);
> >         }
> >         */
>
> You can just call device_destroy() here, but this should be the same.

There's a key difference:

void device_destroy(struct class *class, dev_t devt)
{
    struct device *dev;

    dev = class_find_device_by_devt(class, devt);
    if (dev) {
        put_device(dev);
        device_unregister(dev);
    }
}

It won't work because it receives a dev_t, which in my case is 0,0 and
won't find it.

>
> But, if you have it commented out, that's not good, you have to clean
> this up.
>
> >         class_remove_file(&my_class, &class_attr_howareyou);
>
> You don't always have to remove files you create, but it doesn't hurt.
>
> >         class_unregister(&my_class);
>
> class_destroy()?  But this is the same as well, so all is good.
>
> Try running without the above code commented out.

kaboom :) Seems the reference counting went negative

[  514.412418] kernfs: can not remove 'uevent', no directory
[  514.412425] WARNING: CPU: 6 PID: 6203 at fs/kernfs/dir.c:1507
kernfs_remove_by_name_ns+0x8a/0xa0
[  514.412425] Modules linked in: firmware_security_class(OE-) xxx
[  514.412458] CPU: 6 PID: 6203 Comm: rmmod Tainted: G           OE
 5.8.0+ #1
[  514.412458] Hardware name: xxx
[  514.412459] RIP: 0010:kernfs_remove_by_name_ns+0x8a/0xa0
[  514.412460] Code: 00 31 c0 41 5c 41 5d 41 5e 5d c3 48 c7 c7 80 14
93 b1 e8 19 b0 77 00 b8 fe ff ff ff eb e5 48 c7 c7 f8 56 5d b1 e8 3b
ca ce ff <0f> 0b b8 fe ff ff ff eb d0 66 66 2e 0f 1f 84 00 00 00 00 00
66 90
[  514.412460] RSP: 0018:ffffbedac1a43d48 EFLAGS: 00010286
[  514.412461] RAX: 0000000000000000 RBX: ffffa081de519c00 RCX: 0000000000000027
[  514.412461] RDX: 0000000000000027 RSI: 0000000000000082 RDI: ffffa0823d798cc8
[  514.412462] RBP: ffffbedac1a43d60 R08: 000000000000050e R09: 0000000000000004
[  514.412462] R10: 0000000000000000 R11: 0000000000000001 R12: ffffa082384650b0
[  514.412462] R13: ffffffffb15f9575 R14: dead000000000122 R15: dead000000000100
[  514.412463] FS:  00007f912a4f6540(0000) GS:ffffa0823d780000(0000)
knlGS:0000000000000000
[  514.412464] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  514.412464] CR2: 0000559b849611b8 CR3: 000000080650e001 CR4: 00000000003606e0
[  514.412464] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  514.412465] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  514.412465] Call Trace:
[  514.412468]  sysfs_remove_file_ns+0x15/0x20
[  514.412470]  device_del+0x164/0x3e0
[  514.412472]  device_unregister+0x1b/0x60
[  514.412473]  mypci_remove+0x26/0x50 [firmware_security_class]
[  514.412475]  pci_device_remove+0x3e/0xb0
[  514.412476]  device_release_driver_internal+0xf0/0x1c0
[  514.412477]  driver_detach+0x4c/0x8f
[  514.412477]  bus_remove_driver+0x5c/0xd0
[  514.412478]  driver_unregister+0x31/0x50
[  514.412479]  pci_unregister_driver+0x40/0x90
[  514.412480]  my_pci_driver_exit+0x10/0xe80 [firmware_security_class]
[  514.412482]  __x64_sys_delete_module+0x146/0x2a0
[  514.412483]  ? __syscall_return_slowpath+0x31/0x160
[  514.412485]  do_syscall_64+0x52/0xc0
[  514.412486]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  514.412487] RIP: 0033:0x7f912a642a3b
[  514.412488] Code: 73 01 c3 48 8b 0d 55 84 0c 00 f7 d8 64 89 01 48
83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00
00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 25 84 0c 00 f7 d8 64 89
01 48
[  514.412489] RSP: 002b:00007ffc3e5c6b58 EFLAGS: 00000206 ORIG_RAX:
00000000000000b0
[  514.412489] RAX: ffffffffffffffda RBX: 0000559b84956770 RCX: 00007f912a642a3b
[  514.412490] RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000559b849567d8
[  514.412490] RBP: 00007ffc3e5c6bb8 R08: 0000000000000000 R09: 0000000000000000
[  514.412491] R10: 00007f912a6beac0 R11: 0000000000000206 R12: 00007ffc3e5c6d90
[  514.412491] R13: 00007ffc3e5c77ba R14: 0000559b849562a0 R15: 0000559b84956770
[  514.412492] ---[ end trace 6c3ef2924fc0afa7 ]---
[  514.412492] ------------[ cut here ]------------
[  514.412493] kernfs: can not remove 'online', no directory
[  514.412496] WARNING: CPU: 6 PID: 6203 at fs/kernfs/dir.c:1507
kernfs_remove_by_name_ns+0x8a/0xa0
[  514.412496] Modules linked in: firmware_security_class(OE-) xxx
[  514.412513] CPU: 6 PID: 6203 Comm: rmmod Tainted: G        W  OE
 5.8.0+ #1
[  514.412513] Hardware name: xxx
[  514.412513] RIP: 0010:kernfs_remove_by_name_ns+0x8a/0xa0
[  514.412514] Code: 00 31 c0 41 5c 41 5d 41 5e 5d c3 48 c7 c7 80 14
93 b1 e8 19 b0 77 00 b8 fe ff ff ff eb e5 48 c7 c7 f8 56 5d b1 e8 3b
ca ce ff <0f> 0b b8 fe ff ff ff eb d0 66 66 2e 0f 1f 84 00 00 00 00 00
66 90
[  514.412514] RSP: 0018:ffffbedac1a43d20 EFLAGS: 00010282
[  514.412515] RAX: 0000000000000000 RBX: ffffffffc0c83120 RCX: 0000000000000027
[  514.412515] RDX: 0000000000000027 RSI: 0000000000000092 RDI: ffffa0823d798cc8
[  514.412516] RBP: ffffbedac1a43d38 R08: 000000000000053b R09: 0000000000000004
[  514.412516] R10: 0000000000000000 R11: 0000000000000001 R12: ffffa081de519c00
[  514.412516] R13: ffffffffb16650b8 R14: dead000000000122 R15: dead000000000100
[  514.412517] FS:  00007f912a4f6540(0000) GS:ffffa0823d780000(0000)
knlGS:0000000000000000
[  514.412517] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  514.412518] CR2: 0000559b849611b8 CR3: 000000080650e001 CR4: 00000000003606e0
[  514.412518] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  514.412519] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  514.412519] Call Trace:
[  514.412520]  sysfs_remove_file_ns+0x15/0x20
[  514.412521]  device_remove_attrs+0x2f/0x70
[  514.412521]  device_del+0x173/0x3e0
[  514.412523]  device_unregister+0x1b/0x60
[  514.412523]  mypci_remove+0x26/0x50 [firmware_security_class]
[  514.412524]  pci_device_remove+0x3e/0xb0
[  514.412525]  device_release_driver_internal+0xf0/0x1c0
[  514.412526]  driver_detach+0x4c/0x8f
[  514.412526]  bus_remove_driver+0x5c/0xd0
[  514.412527]  driver_unregister+0x31/0x50
[  514.412528]  pci_unregister_driver+0x40/0x90
[  514.412529]  my_pci_driver_exit+0x10/0xe80 [firmware_security_class]
[  514.412530]  __x64_sys_delete_module+0x146/0x2a0
[  514.412531]  ? __syscall_return_slowpath+0x31/0x160
[  514.412532]  do_syscall_64+0x52/0xc0
[  514.412533]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  514.412533] RIP: 0033:0x7f912a642a3b
[  514.412534] Code: 73 01 c3 48 8b 0d 55 84 0c 00 f7 d8 64 89 01 48
83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00
00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 25 84 0c 00 f7 d8 64 89
01 48
[  514.412534] RSP: 002b:00007ffc3e5c6b58 EFLAGS: 00000206 ORIG_RAX:
00000000000000b0
[  514.412535] RAX: ffffffffffffffda RBX: 0000559b84956770 RCX: 00007f912a642a3b
[  514.412535] RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000559b849567d8
[  514.412535] RBP: 00007ffc3e5c6bb8 R08: 0000000000000000 R09: 0000000000000000
[  514.412536] R10: 00007f912a6beac0 R11: 0000000000000206 R12: 00007ffc3e5c6d90
[  514.412536] R13: 00007ffc3e5c77ba R14: 0000559b849562a0 R15: 0000559b84956770
[  514.412537] ---[ end trace 6c3ef2924fc0afa8 ]---
[  514.412543] ------------[ cut here ]------------
[  514.412543] refcount_t: underflow; use-after-free.
[  514.412548] WARNING: CPU: 6 PID: 6203 at lib/refcount.c:28
refcount_warn_saturate+0xae/0xf0
[  514.412548] Modules linked in: firmware_security_class(OE-) xxx
[  514.412564] CPU: 6 PID: 6203 Comm: rmmod Tainted: G        W  OE
 5.8.0+ #1
[  514.412565] Hardware name: xxx
[  514.412566] RIP: 0010:refcount_warn_saturate+0xae/0xf0
[  514.412566] Code: 65 0c 2d 01 01 e8 77 0a b4 ff 0f 0b 5d c3 80 3d
52 0c 2d 01 00 75 91 48 c7 c7 40 6b 5f b1 c6 05 42 0c 2d 01 01 e8 57
0a b4 ff <0f> 0b 5d c3 80 3d 30 0c 2d 01 00 0f 85 6d ff ff ff 48 c7 c7
98 6b
[  514.412566] RSP: 0018:ffffbedac1a43d90 EFLAGS: 00010286
[  514.412567] RAX: 0000000000000000 RBX: ffffa08238465000 RCX: 0000000000000027
[  514.412567] RDX: 0000000000000027 RSI: 0000000000000096 RDI: ffffa0823d798cc8
[  514.412568] RBP: ffffbedac1a43d90 R08: 0000000000000569 R09: 0000000000000004
[  514.412568] R10: 0000000000000000 R11: 0000000000000001 R12: ffffa081de519c00
[  514.412568] R13: ffffffffc0c83000 R14: dead000000000122 R15: dead000000000100
[  514.412569] FS:  00007f912a4f6540(0000) GS:ffffa0823d780000(0000)
knlGS:0000000000000000
[  514.412569] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  514.412570] CR2: 0000559b849611b8 CR3: 000000080650e001 CR4: 00000000003606e0
[  514.412570] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  514.412570] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  514.412571] Call Trace:
[  514.412572]  kobject_put+0xf1/0x190
[  514.412573]  device_unregister+0x28/0x60
[  514.412574]  mypci_remove+0x26/0x50 [firmware_security_class]
[  514.412575]  pci_device_remove+0x3e/0xb0
[  514.412576]  device_release_driver_internal+0xf0/0x1c0
[  514.412577]  driver_detach+0x4c/0x8f
[  514.412577]  bus_remove_driver+0x5c/0xd0
[  514.412578]  driver_unregister+0x31/0x50
[  514.412579]  pci_unregister_driver+0x40/0x90
[  514.412580]  my_pci_driver_exit+0x10/0xe80 [firmware_security_class]
[  514.412581]  __x64_sys_delete_module+0x146/0x2a0
[  514.412582]  ? __syscall_return_slowpath+0x31/0x160
[  514.412583]  do_syscall_64+0x52/0xc0
[  514.412584]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  514.412584] RIP: 0033:0x7f912a642a3b
[  514.412585] Code: 73 01 c3 48 8b 0d 55 84 0c 00 f7 d8 64 89 01 48
83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00
00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 25 84 0c 00 f7 d8 64 89
01 48
[  514.412585] RSP: 002b:00007ffc3e5c6b58 EFLAGS: 00000206 ORIG_RAX:
00000000000000b0
[  514.412586] RAX: ffffffffffffffda RBX: 0000559b84956770 RCX: 00007f912a642a3b
[  514.412586] RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000559b849567d8
[  514.412586] RBP: 00007ffc3e5c6bb8 R08: 0000000000000000 R09: 0000000000000000
[  514.412587] R10: 00007f912a6beac0 R11: 0000000000000206 R12: 00007ffc3e5c6d90
[  514.412587] R13: 00007ffc3e5c77ba R14: 0000559b849562a0 R15: 0000559b84956770
[  514.412588] ---[ end trace 6c3ef2924fc0afa9 ]---



>
> greg k-h



-- 
Daniel Gutson
Argentina Site Director
Enginieering Director
Eclypsium

Below The Surface: Get the latest threat research and insights on
firmware and supply chain threats from the research team at Eclypsium.
https://eclypsium.com/research/#threatreport