Re: [PATCH] regulator: tps65219: Fix devm_kmalloc size allocation

[Andrew Davis]

On 6/19/25 7:09 PM, Shree Ramamoorthy wrote:
> In probe(), devm_kmalloc uses pmic->common_irq_size to allocate an array of
> 2 bytes, but pmic->common_irq_size is being used like an array of structs.

The issue is with both `common_irq_size` and `dev_irq_size`, so might be good
to mention both since you fix both in the patch. Also "2" bytes was an example,
to be more technically correct (the best kind of correct) you can say:

In probe(), two arrays of structs are allocated but the memory size of the
allocations were given as the arrays' length, not the total memory needed.

> The param sent should've been pmic->common_irq_size * sizeof(*irq_data).
> This led to an issue with the kmalloc'd buffer being corrupted and EVM boot

Don't need to mention "EVM" here unless you want to give a specific example
by name (PocketBeagle2 being the board that first exposed this issue IIRC).

Anyway, the issue wasn't just the kmalloc'd buffer being corrupted, but
other structures in the heap after, and since the issues caused by heap
overflows are usually random, easier to say:

This led to a heap overflow when the struct array was used.

> issues. The common and device-specific structs are now allocated one at a
> time within the loop.

Acked-by: Andrew Davis <afd@xxxxxx>

> Fixes: 38c9f98db20a ("regulator: tps65219: Add support for TPS65215 Regulator IRQs")
> Reported-by: Dhruva Gole <d-gole@xxxxxx>
> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@xxxxxx>
> ---
>   drivers/regulator/tps65219-regulator.c | 28 +++++++++++++-------------
>   1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/regulator/tps65219-regulator.c b/drivers/regulator/tps65219-regulator.c
> index d80749cdae1d..d77ca486879f 100644
> --- a/drivers/regulator/tps65219-regulator.c
> +++ b/drivers/regulator/tps65219-regulator.c
> @@ -436,46 +436,46 @@ static int tps65219_regulator_probe(struct platform_device *pdev)
>   					     pmic->rdesc[i].name);
>   	}
>   
> -	irq_data = devm_kmalloc(tps->dev, pmic->common_irq_size, GFP_KERNEL);
> -	if (!irq_data)
> -		return -ENOMEM;
> -
>   	for (i = 0; i < pmic->common_irq_size; ++i) {
>   		irq_type = &pmic->common_irq_types[i];
>   		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
>   		if (irq < 0)
>   			return -EINVAL;
>   
> -		irq_data[i].dev = tps->dev;
> -		irq_data[i].type = irq_type;
> +		irq_data = devm_kmalloc(tps->dev, sizeof(*irq_data), GFP_KERNEL);
> +		if (!irq_data)
> +			return -ENOMEM;
> +
> +		irq_data->dev = tps->dev;
> +		irq_data->type = irq_type;
>   		error = devm_request_threaded_irq(tps->dev, irq, NULL,
>   						  tps65219_regulator_irq_handler,
>   						  IRQF_ONESHOT,
>   						  irq_type->irq_name,
> -						  &irq_data[i]);
> +						  irq_data);
>   		if (error)
>   			return dev_err_probe(tps->dev, error,
>   					     "Failed to request %s IRQ %d\n",
>   					     irq_type->irq_name, irq);
>   	}
>   
> -	irq_data = devm_kmalloc(tps->dev, pmic->dev_irq_size, GFP_KERNEL);
> -	if (!irq_data)
> -		return -ENOMEM;
> -
>   	for (i = 0; i < pmic->dev_irq_size; ++i) {
>   		irq_type = &pmic->irq_types[i];
>   		irq = platform_get_irq_byname(pdev, irq_type->irq_name);
>   		if (irq < 0)
>   			return -EINVAL;
>   
> -		irq_data[i].dev = tps->dev;
> -		irq_data[i].type = irq_type;
> +		irq_data = devm_kmalloc(tps->dev, sizeof(*irq_data), GFP_KERNEL);
> +		if (!irq_data)
> +			return -ENOMEM;
> +
> +		irq_data->dev = tps->dev;
> +		irq_data->type = irq_type;
>   		error = devm_request_threaded_irq(tps->dev, irq, NULL,
>   						  tps65219_regulator_irq_handler,
>   						  IRQF_ONESHOT,
>   						  irq_type->irq_name,
> -						  &irq_data[i]);
> +						  irq_data);
>   		if (error)
>   			return dev_err_probe(tps->dev, error,
>   					     "Failed to request %s IRQ %d\n",
>
> base-commit: 5c8013ae2e86ec36b07500ba4cacb14ab4d6f728
> prerequisite-patch-id: cd76c901948780de20e932cf620806959e2177b1
> prerequisite-patch-id: e847098a38d07e5ff31e8c80d86d9702d132fdad
> prerequisite-patch-id: e6a01f111e527c6da442f6756f8daa4e79d0fa3c