Re: [PATCH v9 4/8] mtd: spi-nor: sfdp: expose the SFDP as a read-only NVMEM device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: sfdp: steals parent's nvmem-layout when 'jedec,sfdp' node is missing
--

commit e4111fea7a3c70b0bb1f5cd153a259e4fdb62c93
Author: Manikandan Muralidharan <manikandan.m@microchip.com>

mtd: spi-nor: sfdp: expose the SFDP as a read-only NVMEM device

This commit exposes the cached SFDP table as a read-only NVMEM device,
rooted at the flash's SFDP child node if available. This allows generic
and vendor NVMEM cells to reference SFDP data.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 8bc117b46e028..ed478e48ca116 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c

[ ... ]

> @@ -3001,6 +3002,72 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)

[ ... ]

> +static int spi_nor_register_sfdp_nvmem(struct spi_nor *nor)
> +{
> +	struct device *dev = nor->dev;
> +	struct nvmem_config config = { };
> +	struct nvmem_device *nvmem;
> +	struct device_node *np;
> +
> +	if (!nor->sfdp)
> +		return 0;
> +
> +	np = of_get_compatible_child(dev_of_node(dev), "jedec,sfdp");
> +
> +	config.dev = dev;
> +	config.of_node = np;

[Severity: High]
If the jedec,sfdp child node is missing from the device tree, np will
be NULL and assigned to config.of_node. Does this inadvertently trigger the
NVMEM core's fallback behavior?

Looking at nvmem_register() in drivers/nvmem/core.c, there is a fallback
mechanism for missing nodes:

    if (config->of_node)
            nvmem->dev.of_node = of_node_get(config->of_node);
    else
            nvmem->dev.of_node = of_node_get(config->dev->of_node);

If config.of_node is NULL, the SFDP NVMEM device will incorrectly inherit
the SPI NOR flash's main device node.

The NVMEM core then searches for an nvmem-layout child node and will
find the layout intended for the main flash memory, attaching all NVMEM
cells to the small SFDP NVMEM device and marking the layout node populated.

Later, when the MTD subsystem registers the primary NVMEM device for the
actual flash contents, it will skip the layout node because it is already
marked populated. 

Could any consumer attempting to read an NVMEM cell then be misdirected 
to the small SFDP NVMEM device's read callback instead of the main flash, 
resulting in -EINVAL bounds check errors or corrupted data?

> +	config.name = "sfdp";
> +	config.id = NVMEM_DEVID_AUTO;
> +	config.owner = THIS_MODULE;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792145382.git.manikandan.m@microchip.com?part=4