Re: [PATCH v5 3/4] nvmem: mtk-efuse: add CPU speedbin post-processing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] nvmem: flawed string matching applies cpu-speedbin hook to unintended prefixes
- [High] nvmem: out-of-bounds access on zero-length cell in mtk_efuse_cpu_speedbin_pp()
--

commit 6fd2c8ebf274a3effe09809dbfe826b5c25b063f
Author: Roman Vivchar <rva333@protonmail.com>

nvmem: mtk-efuse: add CPU speedbin post-processing

This commit adds support for converting CPU speedbin values stored in efuse
into a bitmask for opp-supported-hw usage in device trees on mobile SoCs.

> diff --git a/drivers/nvmem/mtk-efuse.c b/drivers/nvmem/mtk-efuse.c
> index 980c29d013375..d2e5c69c580e3 100644
> --- a/drivers/nvmem/mtk-efuse.c
> +++ b/drivers/nvmem/mtk-efuse.c
> @@ -70,6 +70,18 @@ static int mtk_efuse_gpu_speedbin_pp(void *context, const char *id, int index,
>  	return 0;
>  }
>  
> +static int mtk_efuse_cpu_speedbin_pp(void *context, const char *id, int index,
> +				     unsigned int offset, void *data, size_t bytes)
> +{
> +	u8 *val = data;
> +
> +	if (val[0] >= 8)

[Severity: High]
If a device tree defines a matching nvmem cell with a length of 0 bytes
(e.g., reg = <0x0 0x0>), the nvmem core allocates an empty buffer using
kzalloc(0, GFP_KERNEL), which returns the ZERO_SIZE_PTR macro.

Could this unconditionally dereference that ZERO_SIZE_PTR and cause a
kernel panic when reading the cell?

> +		return -EINVAL;
> +
> +	val[0] = BIT(val[0]);
> +	return 0;
> +}
> +
>  static void mtk_efuse_fixup_dt_cell_info(struct nvmem_device *nvmem,
>  					 struct nvmem_cell_info *cell)
>  {

[ ... ]

> @@ -83,6 +95,13 @@ static void mtk_efuse_fixup_dt_cell_info(struct nvmem_device *nvmem,
>  	if (cell->nbits <= 3 &&
>  	    strncmp(cell->name, "gpu-speedbin", min(sz, strlen("gpu-speedbin"))) == 0)
>  		cell->read_post_process = mtk_efuse_gpu_speedbin_pp;
> +
> +	/*
> +	 * On mobile SoCs, the CPU speedbin is a number. Convert it to
> +	 * a bitmask for supported-hw usage.
> +	 */
> +	if (strncmp(cell->name, "cpu-speedbin", min(sz, strlen("cpu-speedbin"))) == 0)

[Severity: High]
Does this string comparison correctly restrict the match to just the
"cpu-speedbin" cell?

Since the comparison length uses min(sz, strlen("cpu-speedbin")), if a
device tree contains a cell with a name that is a shorter prefix of
"cpu-speedbin" (like "cpu" or "c"), the min() function will select the
shorter string's length.

Could this result in strncmp returning 0 for unrelated cell prefixes,
incorrectly applying the post-processing hook and silently corrupting
their data?

> +		cell->read_post_process = mtk_efuse_cpu_speedbin_pp;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-6572-nvmem-v5-0-384d2a3a1770@protonmail.com?part=3