Re: [PATCH 3/4] nvmem: layouts: Add Rockchip OTP CPUID layout driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but module unload leaves a dangling function pointer in the nvmem core, leading to execution of unmapped memory when the cell is read.
--

commit eb43a3b0551c59093ae8bd5ad7028542c1ba58bd
Author: Alexey Charkov <alchark@flipper.net>

    nvmem: layouts: Add Rockchip OTP CPUID layout driver
    
    Rockchip SoCs customarily use the CPU ID programmed into their on-chip OTP
    memory to derive stable Ethernet MAC addresses even when a board doesn't
    otherwise have dedicated storage for those.
    
    Reproduce the same derivation in the kernel and expose the result as an
    nvmem cell named "mac-address", so that of_get_mac_address() picks it up
    through its standard nvmem fallback.

> diff --git a/drivers/nvmem/layouts/rockchip-otp-cpuid.c b/drivers/nvmem/layouts/rockchip-otp-cpuid.c
> new file mode 100644
> index 0000000000000..61f509c4a8cf2
> --- /dev/null
> +++ b/drivers/nvmem/layouts/rockchip-otp-cpuid.c

[ ... ]

> @@ -62,15 +62,15 @@ static int rockchip_cpuid_add_cells(struct nvmem_layout *layout)
>  
>  	info.name = "mac-address";
>  	info.offset = data->offset;
>  	info.raw_len = ROCKCHIP_CPUID_LEN;
>  	info.bytes = ETH_ALEN;
>  	info.read_post_process = rockchip_cpuid_mac_pp;
>  	info.np = of_get_child_by_name(layout_np, info.name);
>  
>  	of_node_put(layout_np);
>  
>  	ret = nvmem_add_one_cell(layout->nvmem, &info);
>  	if (ret)
>  		of_node_put(info.np);
>  
>  	return ret;
>  }

[ ... ]

>  static void rockchip_cpuid_remove(struct nvmem_layout *layout)
>  {
>  	nvmem_layout_unregister(layout);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does unloading this module leave
a dangling function pointer in the nvmem core?

In rockchip_cpuid_add_cells() in drivers/nvmem/layouts/rockchip-otp-cpuid.c,
the module's function pointer is handed over to the core cell structure:

	info.read_post_process = rockchip_cpuid_mac_pp;
	ret = nvmem_add_one_cell(layout->nvmem, &info);

When the module is unloaded, rockchip_cpuid_remove() calls
nvmem_layout_unregister(), but that function is an empty stub in the nvmem
framework. As a result, it silently fails to remove the dynamically allocated
cells.

Because the cell outlives the module without a module reference protecting
the layout driver, a subsequent read of the cell (e.g., via sysfs) will
invoke the freed rockchip_cpuid_mac_pp() callback.

Could this lead to execution of unmapped memory and a kernel panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-rk3576-otp-cpuid-mac-v1-0-ea9135270fc2@flipper.net?part=3