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

[sashiko-bot@kernel.org]

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