[PATCH RFC 0/5] nvmem: Add a UEFI variable backed NVMEM provider

[PATCH RFC 0/5] nvmem: Add a UEFI variable backed NVMEM provider

[Abel Vesa]

This is an RFC: I do not expect it to be merged as-is, it is sent to get
feedback on the approach.

Some values a platform needs (for example the WLAN and Bluetooth MAC
addresses on the Qualcomm laptops) are not always available to the kernel
in a convenient form. They may live in firmware-owned SPI-NOR storage
(e.g. a DPP partition) for which there is no straightforward, universally
working way to read them from the non-secure world.

As an alternative, this series allows such values to be provisioned once
into a UEFI variable and consumed in the kernel. It adds a generic NVMEM
provider that exposes the contents of a UEFI variable as an NVMEM device,
so the data can be consumed through the standard nvmem-cells bindings.
MAC addresses are just the motivating use case; the provider itself is
generic.

The first two patches are the actual proposal:

  - dt-bindings: nvmem: Add UEFI variable NVMEM provider
  - nvmem: Add UEFI variable NVMEM provider

The remaining three patches are included only to illustrate how a
consumer would use the provider; they are not meant for merging as part
of this series:

  - Bluetooth: hci_qca: consume a "bd-address" nvmem cell
  - wifi: ath12k: consume a "mac-address" cell
  - arm64: dts: qcom: glymur-crd: wire both to a "MacAddresses" variable

glymur-crd is used here only as a convenient example for the consumer
patches.

Feedback on the provider, and on whether sourcing such values from a UEFI
variable is a reasonable approach when they cannot be read from the
SPI-NOR directly, is very welcome.

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
Abel Vesa (5):
      dt-bindings: nvmem: Add UEFI variable NVMEM provider
      nvmem: Add UEFI variable NVMEM provider
      Bluetooth: hci_qca: Support BD address from an nvmem cell
      wifi: ath12k: Support MAC address from the firmware node
      arm64: dts: qcom: glymur-crd: Source MAC addresses from a UEFI variable

 .../devicetree/bindings/nvmem/nvmem-efivar.yaml    |  73 ++++++
 arch/arm64/boot/dts/qcom/glymur-crd.dtsi           |  22 ++
 drivers/bluetooth/hci_qca.c                        |  48 ++++
 drivers/net/wireless/ath/ath12k/mac.c              |  14 +-
 drivers/nvmem/Kconfig                              |  12 +
 drivers/nvmem/Makefile                             |   2 +
 drivers/nvmem/efivar.c                             | 273 +++++++++++++++++++++
 7 files changed, 443 insertions(+), 1 deletion(-)
---
base-commit: 1fffefb00cc0b0da1f2ae24e3d33364530ea6515
change-id: 20261014-b4-nvmem-efivar-fec3cab05755

Best regards,
--  
Abel Vesa <abel.vesa@oss.qualcomm.com>

[PATCH RFC 1/5] dt-bindings: nvmem: Add UEFI variable NVMEM provider

[Abel Vesa]

Describe a provider that exposes the contents of a UEFI variable as an
NVMEM device, so data provisioned in a UEFI variable can be consumed
through the standard nvmem-cells bindings. The vendor GUID is optional;
when omitted the variable is looked up by name across the whole variable
store. If the variable has not been provisioned, the cells read as
zeroes.

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 .../devicetree/bindings/nvmem/nvmem-efivar.yaml    | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml
new file mode 100644
index 000000000000..cdc10bee3c6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/nvmem-efivar.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: UEFI Variable Based nvmem Device
+
+maintainers:
+  - Abel Vesa <abel.vesa@oss.qualcomm.com>
+
+description: |
+  Exposes the contents of a UEFI variable as an NVMEM device, so that data
+  provisioned in a UEFI variable can be consumed through the standard
+  "nvmem-cells" / "nvmem-cell-names" bindings.
+
+  The variable is read via the EFI runtime GetVariable service, which keeps
+  working even on platforms where that service is emulated by firmware (for
+  example the Qualcomm uefisecapp backend) and where direct flash access is not
+  available to the non-secure world. If the variable has not been provisioned,
+  the cells read as zeroes.
+
+allOf:
+  - $ref: nvmem.yaml#
+  - $ref: nvmem-deprecated-cells.yaml#
+
+properties:
+  compatible:
+    const: nvmem-efivar
+
+  efivar-name:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Name of the UEFI variable to read.
+
+  efivar-guid:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Vendor GUID of the UEFI variable, in the canonical
+      "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" form. Optional; when omitted
+      the variable is looked up by name across the whole variable store.
+
+required:
+  - compatible
+  - efivar-name
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    nvmem-efivar {
+        compatible = "nvmem-efivar";
+        efivar-name = "ConfigData";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        cell_a: cell@0 {
+            reg = <0x0 0x6>;
+        };
+
+        cell_b: cell@6 {
+            reg = <0x6 0x6>;
+        };
+    };
+
+    /* A consumer references a cell through the standard nvmem bindings:
+     *
+     * &some_device {
+     *     nvmem-cells = <&cell_a>;
+     *     nvmem-cell-names = "some-value";
+     * };
+     */
+...

-- 
2.54.0

Re: [PATCH RFC 1/5] dt-bindings: nvmem: Add UEFI variable NVMEM provider

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: nvmem: use of deprecated direct cell syntax
- [Medium] dt-bindings: nvmem: efivar-guid omission allows variable spoofing
- [Low] dt-bindings: missing pattern constraint for efivar-guid
--

commit 648d72d2afdafd63b69b67763f6747f4034d85b2
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

dt-bindings: nvmem: Add UEFI variable NVMEM provider

This commit introduces a new device tree binding for an NVMEM provider that
exposes the contents of UEFI variables. It allows provisioned UEFI data to
be consumed via standard nvmem-cells bindings, optionally looking up the
variable by name if the vendor GUID is omitted.

> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml b/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml
> new file mode 100644
> index 0000000000000..cdc10bee3c6ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem-efivar.yaml
[ ... ]
> +allOf:
> +  - $ref: nvmem.yaml#
> +  - $ref: nvmem-deprecated-cells.yaml#

[Severity: Medium]
Does this binding use the deprecated direct cell syntax?

The NVMEM subsystem guidelines state that defining fixed cells as direct
subnodes is deprecated and that no new bindings should use it. New NVMEM
providers should use the fixed-layout standard with an nvmem-layout child
node containing the cells instead.

[ ... ]
> +  efivar-guid:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Vendor GUID of the UEFI variable, in the canonical

[Severity: Low]
Would it be better to add a pattern constraint to enforce the documented
canonical format?

The schema validation will currently accept arbitrary strings since it
relies on the generic string definition without a regex pattern. Adding a
constraint such as "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"
would prevent malformed GUID strings from passing validation.

> +      "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" form. Optional; when omitted
> +      the variable is looked up by name across the whole variable store.

[Severity: Medium]
Does making the GUID optional introduce a risk of variable spoofing?

The UEFI specification states that variables are uniquely identified only by
the (Name, GUID) tuple. If an attacker with local write access creates a
variable with the same Name but under an unauthenticated GUID, could the
implementation return the spoofed variable depending on firmware enumeration
order?

[ ... ]
> +examples:
> +  - |
> +    nvmem-efivar {
> +        compatible = "nvmem-efivar";
> +        efivar-name = "ConfigData";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        cell_a: cell@0 {
> +            reg = <0x0 0x6>;
> +        };
> +
> +        cell_b: cell@6 {
> +            reg = <0x6 0x6>;
> +        };

[Severity: Medium]
Does this example propagate the deprecated direct subnode anti-pattern?

As noted above regarding nvmem-deprecated-cells.yaml, new NVMEM providers
should demonstrate the use of the fixed-layout standard with an nvmem-layout
child node instead of defining fixed cells as direct subnodes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com?part=1

[PATCH RFC 2/5] nvmem: Add UEFI variable NVMEM provider

[Abel Vesa]

Add a provider that reads a UEFI variable via the EFI runtime
GetVariable service and publishes its bytes as a read-only NVMEM device.
NVMEM cells are described as device-tree child nodes and can be
referenced by consumers through nvmem-cells / nvmem-cell-names.

The vendor GUID is optional: when it is not provided, the variable is
located by name by enumerating the variable store. This keeps working
on platforms where GetVariable is emulated by firmware (e.g. the
Qualcomm uefisecapp backend) and where direct flash access is
unavailable to the non-secure world, in particular when Linux boots at
EL2.

If the variable has not been provisioned yet, the device is still
registered, sized to cover the described cells and filled with zeroes,
the same way an erased EEPROM would read. This keeps consumers probing
and falling back to their default addresses.

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/nvmem/Kconfig  |  12 +++
 drivers/nvmem/Makefile |   2 +
 drivers/nvmem/efivar.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 287 insertions(+)

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 083a5e528960..3f619dfa1cf7 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -185,6 +185,18 @@ config NVMEM_BRCM_NVRAM
 	  This driver provides support for Broadcom's NVRAM that can be accessed
 	  using I/O mapping.
 
+config NVMEM_EFIVAR
+	tristate "UEFI variable NVMEM provider"
+	depends on EFI || COMPILE_TEST
+	help
+	  Say y here to expose the contents of a UEFI variable as an NVMEM
+	  device. This allows the kernel to consume data provisioned in a UEFI
+	  variable, referenced from consumers via the standard nvmem-cells
+	  bindings.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called nvmem-efivar.
+
 config NVMEM_IMX_IIM
 	tristate "i.MX IC Identification Module support"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 5e70bad9f0f9..038d3d9ee95c 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -27,6 +27,8 @@ obj-$(CONFIG_NVMEM_BCM_OCOTP)		+= nvmem-bcm-ocotp.o
 nvmem-bcm-ocotp-y			:= bcm-ocotp.o
 obj-$(CONFIG_NVMEM_BRCM_NVRAM)		+= nvmem_brcm_nvram.o
 nvmem_brcm_nvram-y			:= brcm_nvram.o
+obj-$(CONFIG_NVMEM_EFIVAR)		+= nvmem-efivar.o
+nvmem-efivar-y			:= efivar.o
 obj-$(CONFIG_NVMEM_IMX_IIM)		+= nvmem-imx-iim.o
 nvmem-imx-iim-y				:= imx-iim.o
 obj-$(CONFIG_NVMEM_IMX_OCOTP)		+= nvmem-imx-ocotp.o
diff --git a/drivers/nvmem/efivar.c b/drivers/nvmem/efivar.c
new file mode 100644
index 000000000000..699b1d67aea4
--- /dev/null
+++ b/drivers/nvmem/efivar.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * nvmem-efivar - expose the contents of a UEFI variable as an NVMEM device
+ *
+ * Lets the kernel consume data provisioned in a UEFI variable through the
+ * standard NVMEM cell bindings.  The variable is read at probe time through
+ * the EFI runtime GetVariable service, which also works on platforms where
+ * the variable store is emulated by firmware (for example the Qualcomm
+ * uefisecapp backend) and where direct flash access is not available to the
+ * non-secure world.  The variable contents are published as a read-only
+ * NVMEM device, with cells described as child nodes in the device tree and
+ * referenced by consumers through the standard "nvmem-cells" /
+ * "nvmem-cell-names" properties.
+ *
+ * If the variable has not been provisioned yet, the device is still
+ * registered, sized to cover the described cells and filled with zeroes,
+ * the same way an erased EEPROM would read.  Consumers are expected to
+ * validate the data they read so such systems keep working with their
+ * defaults.
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/efi.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/ucs2_string.h>
+#include <linux/uuid.h>
+
+struct efivar_nvmem {
+	void	*data;
+	size_t	size;
+};
+
+static int efivar_nvmem_reg_read(void *priv, unsigned int offset,
+				 void *val, size_t bytes)
+{
+	struct efivar_nvmem *p = priv;
+
+	if (offset >= p->size || offset + bytes > p->size)
+		return -EINVAL;
+
+	memcpy(val, p->data + offset, bytes);
+	return 0;
+}
+
+/* Size needed to cover all cells described as child nodes. */
+static size_t efivar_nvmem_cells_extent(struct device *dev)
+{
+	struct device_node *child;
+	size_t extent = 0;
+	u32 reg[2];
+
+	for_each_child_of_node(dev->of_node, child) {
+		if (of_property_read_u32_array(child, "reg", reg, 2))
+			continue;
+		extent = max_t(size_t, extent, (size_t)reg[0] + reg[1]);
+	}
+
+	return extent;
+}
+
+/*
+ * Find the vendor GUID of a variable given only its (UCS-2) name, by
+ * enumerating the variable store.  Used when no 'efivar-guid' is provided.
+ */
+static int efivar_nvmem_find_guid(const efi_char16_t *want, efi_guid_t *out)
+{
+	unsigned long max = 512; /* in UCS-2 chars; variable names are short */
+	efi_char16_t *name;
+	efi_guid_t vendor = {};
+	efi_status_t status;
+	int ret = -ENODEV;
+
+	name = kcalloc(max, sizeof(*name), GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+	name[0] = 0;
+
+	for (;;) {
+		unsigned long name_size = max * sizeof(*name);
+
+		status = efivar_get_next_variable(&name_size, name, &vendor);
+		if (status == EFI_BUFFER_TOO_SMALL) {
+			efi_char16_t *tmp;
+
+			max = name_size / sizeof(*name) + 1;
+			tmp = krealloc_array(name, max, sizeof(*name), GFP_KERNEL);
+			if (!tmp) {
+				ret = -ENOMEM;
+				break;
+			}
+			name = tmp;
+			continue;
+		}
+		if (status != EFI_SUCCESS) /* EFI_NOT_FOUND ends the walk */
+			break;
+
+		if (!ucs2_strncmp(name, want, ucs2_strlen(want) + 1)) {
+			*out = vendor;
+			ret = 0;
+			break;
+		}
+	}
+
+	kfree(name);
+	return ret;
+}
+
+static int efivar_nvmem_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const char *name, *guid_str;
+	struct nvmem_config cfg = {};
+	struct efivar_nvmem *priv;
+	struct nvmem_device *nvmem;
+	efi_char16_t *name16;
+	efi_guid_t guid;
+	guid_t g;
+	unsigned long size;
+	efi_status_t status;
+	u32 attr;
+	int i, len, ret;
+	void *buf;
+
+	/*
+	 * The EFI variable service may not be a native EFI runtime service;
+	 * backends such as the Qualcomm uefisecapp driver register the
+	 * efivars operations asynchronously.  Wait for them.
+	 */
+	if (!efivar_is_available())
+		return -EPROBE_DEFER;
+
+	ret = device_property_read_string(dev, "efivar-name", &name);
+	if (ret)
+		return dev_err_probe(dev, ret, "missing 'efivar-name'\n");
+
+	/* ASCII variable name -> UCS-2 as expected by the EFI service */
+	len = strlen(name);
+	name16 = kcalloc(len + 1, sizeof(*name16), GFP_KERNEL);
+	if (!name16)
+		return -ENOMEM;
+	for (i = 0; i < len; i++)
+		name16[i] = name[i];
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		kfree(name16);
+		return -ENOMEM;
+	}
+
+	/*
+	 * The vendor GUID is optional: if it is not provided, look the
+	 * variable up by name across the whole store.  A variable that has
+	 * not been provisioned yet is reported as not found and handled
+	 * below like an erased EEPROM.
+	 */
+	if (!device_property_read_string(dev, "efivar-guid", &guid_str)) {
+		ret = guid_parse(guid_str, &g);
+		if (ret) {
+			kfree(name16);
+			return dev_err_probe(dev, ret, "invalid 'efivar-guid'\n");
+		}
+		memcpy(&guid, &g, sizeof(guid));
+		status = EFI_SUCCESS;
+	} else {
+		ret = efivar_nvmem_find_guid(name16, &guid);
+		if (ret && ret != -ENODEV) {
+			kfree(name16);
+			return dev_err_probe(dev, ret,
+					     "failed to look up variable '%s'\n",
+					     name);
+		}
+		status = ret ? EFI_NOT_FOUND : EFI_SUCCESS;
+	}
+
+	/* First attempt with a modest buffer; grow if the firmware asks. */
+	size = 512;
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf) {
+		kfree(name16);
+		return -ENOMEM;
+	}
+
+	if (status == EFI_SUCCESS) {
+		status = efivar_get_variable(name16, &guid, &attr, &size, buf);
+		if (status == EFI_BUFFER_TOO_SMALL) {
+			kfree(buf);
+			buf = kzalloc(size, GFP_KERNEL);
+			if (!buf) {
+				kfree(name16);
+				return -ENOMEM;
+			}
+			status = efivar_get_variable(name16, &guid, &attr,
+						     &size, buf);
+		}
+	}
+	kfree(name16);
+
+	if (status == EFI_NOT_FOUND) {
+		/*
+		 * Variable not provisioned yet: behave like an erased
+		 * EEPROM so that consumers still probe and fall back to
+		 * their defaults.
+		 */
+		kfree(buf);
+		size = efivar_nvmem_cells_extent(dev);
+		if (!size)
+			return dev_err_probe(dev, -ENODEV,
+					     "variable '%s' not found and no cells described\n",
+					     name);
+		buf = kzalloc(size, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+		dev_info(dev, "variable '%s' not provisioned, reading as zeroes\n",
+			 name);
+	} else if (status != EFI_SUCCESS) {
+		kfree(buf);
+		return dev_err_probe(dev, -ENODEV,
+				     "GetVariable('%s') failed: 0x%lx\n",
+				     name, status);
+	} else {
+		dev_dbg(dev, "variable '%s' attr=0x%x size=%lu\n",
+			name, attr, size);
+	}
+
+	priv->data = devm_kmemdup(dev, buf, size, GFP_KERNEL);
+	kfree(buf);
+	if (!priv->data)
+		return -ENOMEM;
+	priv->size = size;
+
+	cfg.dev			= dev;
+	cfg.name		= "efivar";
+	cfg.id			= NVMEM_DEVID_AUTO;
+	cfg.owner		= THIS_MODULE;
+	cfg.type		= NVMEM_TYPE_OTP;
+	cfg.read_only		= true;
+	cfg.word_size		= 1;
+	cfg.stride		= 1;
+	cfg.size		= priv->size;
+	cfg.reg_read		= efivar_nvmem_reg_read;
+	cfg.priv		= priv;
+	cfg.of_node		= dev->of_node;
+	cfg.add_legacy_fixed_of_cells = true;
+
+	nvmem = devm_nvmem_register(dev, &cfg);
+	return PTR_ERR_OR_ZERO(nvmem);
+}
+
+static const struct of_device_id efivar_nvmem_of_match[] = {
+	{ .compatible = "nvmem-efivar" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, efivar_nvmem_of_match);
+
+static struct platform_driver efivar_nvmem_driver = {
+	.probe = efivar_nvmem_probe,
+	.driver = {
+		.name = "nvmem-efivar",
+		.of_match_table = efivar_nvmem_of_match,
+	},
+};
+module_platform_driver(efivar_nvmem_driver);
+
+MODULE_DESCRIPTION("NVMEM provider backed by a UEFI variable");
+MODULE_IMPORT_NS("EFIVAR");
+MODULE_LICENSE("GPL");

-- 
2.54.0

Re: [PATCH RFC 2/5] nvmem: Add UEFI variable NVMEM provider

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvmem: efivar: missing efivars_lock during EFI variable API calls
--

commit 95924ede9aa4d9717fd0738df67a068299d524f1
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

nvmem: Add UEFI variable NVMEM provider

This commit adds a provider that reads a UEFI variable via the EFI runtime
GetVariable service and publishes its bytes as a read-only NVMEM device.

> diff --git a/drivers/nvmem/efivar.c b/drivers/nvmem/efivar.c
> new file mode 100644
> index 0000000000000..699b1d67aea44
> --- /dev/null
> +++ b/drivers/nvmem/efivar.c

[ ... ]

> @@ -68,26 +68,26 @@ static size_t efivar_nvmem_cells_extent(struct device *dev)
>  /*
>   * Find the vendor GUID of a variable given only its (UCS-2) name, by
>   * enumerating the variable store.  Used when no 'efivar-guid' is provided.
>   */
>  static int efivar_nvmem_find_guid(const efi_char16_t *want, efi_guid_t *out)
>  {
>  	unsigned long max = 512; /* in UCS-2 chars; variable names are short */
>  	efi_char16_t *name;
>  	efi_guid_t vendor = {};
>  	efi_status_t status;
>  	int ret = -ENODEV;
>  
>  	name = kcalloc(max, sizeof(*name), GFP_KERNEL);
>  	if (!name)
>  		return -ENOMEM;
>  	name[0] = 0;
>  
>  	for (;;) {
>  		unsigned long name_size = max * sizeof(*name);
>  
>  		status = efivar_get_next_variable(&name_size, name, &vendor);

[Severity: High]
Does this call to efivar_get_next_variable() require efivars_lock to be held
via efivar_lock()?

As documented in drivers/firmware/efi/vars.c, EFI variable APIs mandate that
efivars_lock is held by the caller. The lock protects the global __efivars
pointer and underlying firmware operations from concurrent state changes.

If the EFI variable backend (such as uefisecapp) is concurrently unregistered
via efivars_unregister(), __efivars is set to NULL under the lock. Without
serialization, the unprotected __efivars->ops->get_next_variable call inside
the EFI core will trigger a NULL pointer dereference.

>  		if (status == EFI_BUFFER_TOO_SMALL) {
>  			efi_char16_t *tmp;

[ ... ]

> @@ -180,24 +180,24 @@ static int efivar_nvmem_probe(struct platform_device *pdev)
>  		status = ret ? EFI_NOT_FOUND : EFI_SUCCESS;
>  	}
>  
>  	/* First attempt with a modest buffer; grow if the firmware asks. */
>  	size = 512;
>  	buf = kzalloc(size, GFP_KERNEL);
>  	if (!buf) {
>  		kfree(name16);
>  		return -ENOMEM;
>  	}
>  
>  	if (status == EFI_SUCCESS) {
>  		status = efivar_get_variable(name16, &guid, &attr, &size, buf);

[Severity: High]
Similar to efivar_get_next_variable() above, should this call to
efivar_get_variable() be protected by efivar_lock()?

Without the lock, an unprotected __efivars->ops->get_variable call inside
the EFI core can trigger a NULL pointer dereference if the backend is
concurrently unregistered.

>  		if (status == EFI_BUFFER_TOO_SMALL) {
>  			kfree(buf);
>  			buf = kzalloc(size, GFP_KERNEL);
>  			if (!buf) {
>  				kfree(name16);
>  				return -ENOMEM;
>  			}
>  			status = efivar_get_variable(name16, &guid, &attr,
>  						     &size, buf);

[Severity: High]
Does this second call to efivar_get_variable() also need to be protected
by efivar_lock()?

>  		}
>  	}
>  	kfree(name16);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com?part=2

[PATCH RFC 3/5] Bluetooth: hci_qca: Support BD address from an nvmem cell

[Abel Vesa]

Read an optional Bluetooth device address from an nvmem cell named
"bd-address" (as already supported by hci_ll), and program it into the
controller during setup. This lets QCA controllers source their BD_ADDR
from firmware-provisioned storage such as a UEFI variable exposed via
nvmem. An all-zero value is ignored, so unprovisioned storage leaves
the controller default address untouched.

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/bluetooth/hci_qca.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 7089e9b639b2..b704f4c3e1a4 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -25,6 +25,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/nvmem-consumer.h>
 #include <linux/acpi.h>
 #include <linux/platform_device.h>
 #include <linux/pwrseq/consumer.h>
@@ -230,6 +231,8 @@ struct qca_serdev {
 	bool bdaddr_property_broken;
 	bool support_hfp_hw_offload;
 	const char *firmware_name[2];
+	bdaddr_t bdaddr;
+	bool bdaddr_valid;
 };
 
 static int qca_regulator_enable(struct qca_serdev *qcadev);
@@ -2079,6 +2082,16 @@ static int qca_setup(struct hci_uart *hu)
 	else
 		hu->hdev->set_bdaddr = qca_set_bdaddr;
 
+	/* Program a BD address sourced from an nvmem cell, if provided. */
+	if (qcadev && qcadev->bdaddr_valid) {
+		int berr = hu->hdev->set_bdaddr(hdev, &qcadev->bdaddr);
+
+		if (berr)
+			bt_dev_warn(hdev, "Failed to set nvmem bd-address: %d", berr);
+		else
+			bt_dev_info(hdev, "Set bd-address %pMR from nvmem", &qcadev->bdaddr);
+	}
+
 	if (qcadev && qcadev->support_hfp_hw_offload)
 		qca_configure_hfp_offload(hdev);
 
@@ -2420,6 +2433,41 @@ static int qca_serdev_probe(struct serdev_device *serdev)
 	qcadev->bdaddr_property_broken = device_property_read_bool(&serdev->dev,
 			"qcom,local-bd-address-broken");
 
+	/* Optional BD address from an nvmem cell (e.g. a UEFI variable). */
+	{
+		struct nvmem_cell *cell;
+
+		cell = nvmem_cell_get(&serdev->dev, "bd-address");
+		if (IS_ERR(cell)) {
+			err = PTR_ERR(cell);
+			if (err == -EPROBE_DEFER)
+				return err;
+			/* -ENOENT/-ENOSYS: no cell configured, ignore. */
+		} else {
+			size_t clen;
+			void *cdata = nvmem_cell_read(cell, &clen);
+
+			nvmem_cell_put(cell);
+			if (!IS_ERR(cdata)) {
+				/*
+				 * Ignore an all-zero address: the backing
+				 * storage has not been provisioned.
+				 */
+				if (clen == sizeof(bdaddr_t) &&
+				    bacmp(cdata, BDADDR_ANY)) {
+					/*
+					 * Per the device tree bindings the nvmem
+					 * value is MSB first, while bdaddr_t is
+					 * LSB first.
+					 */
+					baswap(&qcadev->bdaddr, cdata);
+					qcadev->bdaddr_valid = true;
+				}
+				kfree(cdata);
+			}
+		}
+	}
+
 	if (data)
 		qcadev->btsoc_type = data->soc_type;
 	else

-- 
2.54.0

Re: [PATCH RFC 3/5] Bluetooth: hci_qca: Support BD address from an nvmem cell

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bluetooth: device wrongly left HCI_UNCONFIGURED after nvmem MAC init
--

commit 97b88af275cdce36b0a1523d7d18ad5eb2710216
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

Bluetooth: hci_qca: Support BD address from an nvmem cell

This commit reads an optional Bluetooth device address from an nvmem cell
and programs it into the controller during setup. This allows sourcing
BD_ADDR from firmware-provisioned storage.

> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c

[ ... ]

> @@ -2079,6 +2082,16 @@ static int qca_setup(struct hci_uart *hu)
>  	else
>  		hu->hdev->set_bdaddr = qca_set_bdaddr;
>  
> +	/* Program a BD address sourced from an nvmem cell, if provided. */
> +	if (qcadev && qcadev->bdaddr_valid) {
> +		int berr = hu->hdev->set_bdaddr(hdev, &qcadev->bdaddr);
> +
> +		if (berr)
> +			bt_dev_warn(hdev, "Failed to set nvmem bd-address: %d", berr);
> +		else
> +			bt_dev_info(hdev, "Set bd-address %pMR from nvmem", &qcadev->bdaddr);
> +	}

[Severity: High]
Does this leave the device wrongly flagged as HCI_UNCONFIGURED?

When the controller boots with a default MAC, qca_check_bdaddr() detects it
and sets HCI_QUIRK_USE_BDADDR_PROPERTY. Here in qca_setup(), the nvmem MAC
is programmed into the controller using hu->hdev->set_bdaddr(), but it
appears it is not copied to hdev->public_addr, and
HCI_QUIRK_USE_BDADDR_PROPERTY is not cleared.

When initialization returns to the core stack, hci_dev_setup_sync() sees
HCI_QUIRK_USE_BDADDR_PROPERTY is still set and hdev->public_addr is
BDADDR_ANY:

net/bluetooth/hci_sync.c:hci_dev_setup_sync() {
    ...
    if (invalid_bdaddr && bacmp(&hdev->public_addr, BDADDR_ANY) &&
        hdev->set_bdaddr) {
        ret = hdev->set_bdaddr(hdev, &hdev->public_addr);
        if (!ret)
            invalid_bdaddr = false;
    }
    ...
}

Because hdev->public_addr is still BDADDR_ANY, invalid_bdaddr remains
true.

The core stack then marks the device as HCI_UNCONFIGURED:

net/bluetooth/hci_sync.c:hci_dev_setup_sync() {
    ...
    if (hci_test_quirk(hdev, HCI_QUIRK_EXTERNAL_CONFIG) ||
        invalid_bdaddr)
        hci_dev_set_flag(hdev, HCI_UNCONFIGURED);
    ...
}

Will this cause the core Bluetooth stack to refuse to bring it up
automatically, requiring manual userspace intervention and breaking the
automatic nvmem MAC provisioning?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com?part=3

[PATCH RFC 4/5] wifi: ath12k: Support MAC address from the firmware node

[Abel Vesa]

Read a MAC address from the device's firmware node (a DT "mac-address"/
"local-mac-address" property, or an nvmem cell named "mac-address" via
device_get_mac_address()), and use it in preference to the address from
the firmware board data. This mirrors what ath10k and ath11k already do
and lets the WLAN MAC be sourced from firmware-provisioned storage such
as a UEFI variable exposed through nvmem.

A device tree / nvmem address takes precedence over the board-data MAC,
so an explicitly provisioned address wins even when the firmware reports
a valid per-pdev address. An all-zero (absent) address is ignored.

Note: the provisioned address must be a valid unicast MAC; a multicast
address (odd first octet) is rejected by nvmem_get_mac_address().

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 01f8c6572f65..f4d2d0377c8e 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -14878,15 +14878,27 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 	u32 ht_cap = U32_MAX, antennas_rx = 0, antennas_tx = 0;
 	bool is_6ghz = false, is_raw_mode = false, is_monitor_disable = false;
 	u8 *mac_addr = NULL;
+	u8 mac_from_dt[ETH_ALEN] = {};
 	u8 mbssid_max_interfaces = 0;
 
 	wiphy->max_ap_assoc_sta = 0;
 
+	/*
+	 * Allow a MAC address provided through the device tree (directly or
+	 * via an nvmem cell named "mac-address") to override the one from the
+	 * firmware board data.
+	 */
+	device_get_mac_address(ab->dev, mac_from_dt);
+
 	for_each_ar(ah, ar, i) {
 		u32 ht_cap_info = 0;
 
 		pdev = ar->pdev;
-		if (ar->ab->pdevs_macaddr_valid) {
+		if (!is_zero_ether_addr(mac_from_dt)) {
+			/* A device tree / nvmem address takes precedence. */
+			ether_addr_copy(ar->mac_addr, mac_from_dt);
+			ar->mac_addr[4] += ar->pdev_idx;
+		} else if (ar->ab->pdevs_macaddr_valid) {
 			ether_addr_copy(ar->mac_addr, pdev->mac_addr);
 		} else {
 			ether_addr_copy(ar->mac_addr, ar->ab->mac_addr);

-- 
2.54.0

Re: [PATCH RFC 4/5] wifi: ath12k: Support MAC address from the firmware node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] ath12k: invalid MAC address bypass via ignored device_get_mac_address() return
- [Medium] wifi: ath12k: DT MAC override ignored on multi-radio wiphy
- [Low] ath12k: unaligned memory access risk with mac_from_dt array
--

commit 8b8a36f00c150187c70e8a8b42d0d7b0eecc142a
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

wifi: ath12k: Support MAC address from the firmware node

Read a MAC address from the device's firmware node (a DT "mac-address"/
"local-mac-address" property, or an nvmem cell named "mac-address" via
device_get_mac_address()), and use it in preference to the address from
the firmware board data. This mirrors what ath10k and ath11k already do
and lets the WLAN MAC be sourced from firmware-provisioned storage such
as a UEFI variable exposed through nvmem.

> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 01f8c6572f657..f4d2d0377c8ef 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -14878,15 +14878,27 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
>  	u32 ht_cap = U32_MAX, antennas_rx = 0, antennas_tx = 0;
>  	bool is_6ghz = false, is_raw_mode = false, is_monitor_disable = false;
>  	u8 *mac_addr = NULL;
> +	u8 mac_from_dt[ETH_ALEN] = {};

[Severity: Low]
Does this code risk an unaligned memory access trap?

The mac_from_dt array is declared as u8 without __aligned(2), meaning it
could be placed at an odd address on the stack.

When mac_from_dt is later passed to is_zero_ether_addr() and
ether_addr_copy(), both cast the pointer to u16* (and u32* depending on arch
config), which can trigger alignment traps or performance penalties on
architectures with strict alignment rules.

>  	u8 mbssid_max_interfaces = 0;
>  
>  	wiphy->max_ap_assoc_sta = 0;
>  
> +	/*
> +	 * Allow a MAC address provided through the device tree (directly or
> +	 * via an nvmem cell named "mac-address") to override the one from the
> +	 * firmware board data.
> +	 */
> +	device_get_mac_address(ab->dev, mac_from_dt);

[Severity: Medium]
Could this bypass input validation if device_get_mac_address() fails?

If fwnode_get_mac_address() internal to device_get_mac_address() reads an
invalid MAC address (e.g. multicast), is_valid_ether_addr() inside
fwnode_get_mac_addr() will fail and return -EINVAL.

However, the invalid non-zero bytes are left in mac_from_dt. Because the
error return from device_get_mac_address() is ignored here, the subsequent
check against is_zero_ether_addr(mac_from_dt) will succeed, causing the
invalid address to be applied.

> +
>  	for_each_ar(ah, ar, i) {
>  		u32 ht_cap_info = 0;
>  
>  		pdev = ar->pdev;
> -		if (ar->ab->pdevs_macaddr_valid) {
> +		if (!is_zero_ether_addr(mac_from_dt)) {
> +			/* A device tree / nvmem address takes precedence. */
> +			ether_addr_copy(ar->mac_addr, mac_from_dt);

[Severity: Medium]
Is the device tree override ignored for multi-radio devices?

Later in this same for_each_ar() loop, the permanent MAC address is selected:

	if (i == 0)
		mac_addr = ar->mac_addr;
	else
		mac_addr = ab->mac_addr;

If the device has multiple radios (i > 0), mac_addr defaults to ab->mac_addr.
Because ab->mac_addr is never updated with mac_from_dt,
SET_IEEE80211_PERM_ADDR and ath12k_mac_setup_mac_address_list() will silently
fall back to the un-overridden firmware MAC address for the entire wiphy.

> +			ar->mac_addr[4] += ar->pdev_idx;
> +		} else if (ar->ab->pdevs_macaddr_valid) {
>  			ether_addr_copy(ar->mac_addr, pdev->mac_addr);
>  		} else {
>  			ether_addr_copy(ar->mac_addr, ar->ab->mac_addr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com?part=4

[PATCH RFC 5/5] arm64: dts: qcom: glymur-crd: Source MAC addresses from a UEFI variable

[Abel Vesa]

Add an nvmem-efivar provider exposing the "MacAddresses" UEFI variable,
carrying the Bluetooth BD_ADDR (bytes 0-5) and the WLAN MAC address
(bytes 6-11), both MSB first. The variable is looked up by name, so no
vendor GUID needs to be hardcoded.

Wire the bd-address cell to the Bluetooth controller and the mac-address
cell to the WLAN endpoint (pci17cb,1107) via the standard nvmem-cells
binding. Until the variable is provisioned the cells read as zeroes and
the controllers keep using their default addresses.

Assisted-by: LLM
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
index 58158b51fe29..2dd09ab1d550 100644
--- a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
@@ -31,6 +31,22 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	/* BD_ADDR and WLAN MAC from the "MacAddresses" UEFI variable. */
+	efivar_macs: nvmem-efivar {
+		compatible = "nvmem-efivar";
+		efivar-name = "MacAddresses";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		bt_bdaddr: bd-address@0 {
+			reg = <0x0 0x6>;
+		};
+
+		net_mac: mac-address@6 {
+			reg = <0x6 0x6>;
+		};
+	};
+
 	clocks {
 		xo_board: xo-board {
 			compatible = "fixed-clock";
@@ -776,6 +792,9 @@ wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
 
+		nvmem-cells = <&net_mac>;
+		nvmem-cell-names = "mac-address";
+
 		vddaon-supply = <&vreg_pmu_aon_0p59>;
 		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
 		vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;
@@ -1163,6 +1182,9 @@ bluetooth {
 		compatible = "qcom,wcn7850-bt";
 		max-speed = <3200000>;
 
+		nvmem-cells = <&bt_bdaddr>;
+		nvmem-cell-names = "bd-address";
+
 		vddaon-supply = <&vreg_pmu_aon_0p59>;
 		vddwlcx-supply = <&vreg_pmu_wlcx_0p8>;
 		vddwlmx-supply = <&vreg_pmu_wlmx_0p85>;

-- 
2.54.0

Re: [PATCH RFC 5/5] arm64: dts: qcom: glymur-crd: Source MAC addresses from a UEFI variable

[Konrad Dybcio]

On 10/14/26 3:02 PM, Abel Vesa wrote:
> Add an nvmem-efivar provider exposing the "MacAddresses" UEFI variable,
> carrying the Bluetooth BD_ADDR (bytes 0-5) and the WLAN MAC address
> (bytes 6-11), both MSB first. The variable is looked up by name, so no
> vendor GUID needs to be hardcoded.
> 
> Wire the bd-address cell to the Bluetooth controller and the mac-address
> cell to the WLAN endpoint (pci17cb,1107) via the standard nvmem-cells
> binding. Until the variable is provisioned the cells read as zeroes and
> the controllers keep using their default addresses.
> 
> Assisted-by: LLM
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> index 58158b51fe29..2dd09ab1d550 100644
> --- a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> @@ -31,6 +31,22 @@ chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	/* BD_ADDR and WLAN MAC from the "MacAddresses" UEFI variable. */

Let's drop this comment and sort the node

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH RFC 0/5] nvmem: Add a UEFI variable backed NVMEM provider

[Konrad Dybcio]

On 10/14/26 3:02 PM, Abel Vesa wrote:
> This is an RFC: I do not expect it to be merged as-is, it is sent to get
> feedback on the approach.
> 
> Some values a platform needs (for example the WLAN and Bluetooth MAC
> addresses on the Qualcomm laptops) are not always available to the kernel
> in a convenient form. They may live in firmware-owned SPI-NOR storage
> (e.g. a DPP partition) for which there is no straightforward, universally
> working way to read them from the non-secure world.
> 
> As an alternative, this series allows such values to be provisioned once
> into a UEFI variable and consumed in the kernel. It adds a generic NVMEM
> provider that exposes the contents of a UEFI variable as an NVMEM device,
> so the data can be consumed through the standard nvmem-cells bindings.
> MAC addresses are just the motivating use case; the provider itself is
> generic.
> 
> The first two patches are the actual proposal:
> 
>   - dt-bindings: nvmem: Add UEFI variable NVMEM provider
>   - nvmem: Add UEFI variable NVMEM provider
> 
> The remaining three patches are included only to illustrate how a
> consumer would use the provider; they are not meant for merging as part
> of this series:
> 
>   - Bluetooth: hci_qca: consume a "bd-address" nvmem cell
>   - wifi: ath12k: consume a "mac-address" cell
>   - arm64: dts: qcom: glymur-crd: wire both to a "MacAddresses" variable

I think we can take advantage of Loic's work here

https://lore.kernel.org/linux-arm-msm/20260507-block-as-nvmem-v2-0-bf17edd5134e@oss.qualcomm.com/

Konrad