Re: [PATCH v9 5/8] nvmem: layouts: add Microchip/SST SFDP EUI layout driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvmem: Use-after-free on layout module unload via read_post_process
--

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

nvmem: layouts: add Microchip/SST SFDP EUI layout driver

Add an NVMEM layout that exposes the factory-programmed EUI-48 identifier
from the Microchip/SST vendor SFDP parameter table (e.g. SST26VF064BEUI)
as a "mac-address" cell, for use as a network MAC address.

> diff --git a/drivers/nvmem/layouts/sst26vf-sfdp-eui.c b/drivers/nvmem/layouts/sst26vf-sfdp-eui.c
> new file mode 100644
> index 0000000000000..f46a6d9d0e93a
> --- /dev/null
> +++ b/drivers/nvmem/layouts/sst26vf-sfdp-eui.c
[ ... ]
> +static int sfdp_eui_add_cells(struct nvmem_layout *layout)
> +{
> +	struct nvmem_device *nvmem = layout->nvmem;
[ ... ]
> +	info.name = "mac-address";
> +	info.offset = base + SFDP_MCHP_EUI48_OFFSET;
> +	info.bytes = ETH_ALEN;
> +	info.np = of_get_child_by_name(layout_np, "mac-address");
> +	info.read_post_process = sfdp_eui_read_post_process;
> +
> +	ret = nvmem_add_one_cell(nvmem, &info);
> +	if (ret)
> +		of_node_put(info.np);
> +
> +	of_node_put(layout_np);
> +
> +	return ret;
> +}
[ ... ]
> +static void sfdp_eui_remove(struct nvmem_layout *layout)
> +{
> +	nvmem_layout_unregister(layout);
> +}

[Severity: High]
Does this code leave the module vulnerable to a use-after-free panic on
module unload?

The core nvmem function nvmem_layout_unregister() is an empty stub that does
not remove the registered cells:

drivers/nvmem/core.c:nvmem_layout_unregister() {
	/* Keep the API even with an empty stub in case we need it later */
}

When this layout driver is unloaded, sfdp_eui_remove() calls
nvmem_layout_unregister(layout) but the cells and sysfs attributes are left
dangling.

Because the cell was configured with a read_post_process callback pointing to
sfdp_eui_read_post_process in this module's text segment, a subsequent read of
the sysfs cell file after the module is unloaded will call the dangling
function pointer:

drivers/nvmem/core.c:__nvmem_cell_read() {
    ...
		rc = cell->read_post_process(cell->priv, id, index,
					     cell->offset, buf, cell->raw_len);
    ...
}

Will this cause a kernel panic by executing freed module memory?

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