mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Davis <afd@ti.com>
To: Shree Ramamoorthy <s-ramamoorthy@ti.com>, <aaro.koskinen@iki.fi>,
	<andreas@kemnade.info>, <khilman@baylibre.com>,
	<rogerq@kernel.org>, <tony@atomide.com>, <lee@kernel.org>,
	<d-gole@ti.com>, <robertcnelson@gmail.com>, <jkridner@gmail.com>,
	<linux-omap@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <m-leonard@ti.com>, <praneeth@ti.com>
Subject: Re: [PATCH] regulator: tps65219: Fix devm_kmalloc size allocation
Date: Fri, 20 Jun 2025 09:58:22 -0500	[thread overview]
Message-ID: <67352ec8-6bd0-4434-8553-9fe7829ed188@ti.com> (raw)
In-Reply-To: <20250620000924.2636542-1-s-ramamoorthy@ti.com>

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@ti.com>

> Fixes: 38c9f98db20a ("regulator: tps65219: Add support for TPS65215 Regulator IRQs")
> Reported-by: Dhruva Gole <d-gole@ti.com>
> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com>
> ---
>   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

  parent reply	other threads:[~2025-06-20 14:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-20  0:09 Shree Ramamoorthy
2025-06-20 13:30 ` Robert Nelson
2025-06-20 14:52   ` Robert Nelson
2025-06-20 15:22     ` Shree Ramamoorthy
2025-06-20 15:19   ` Shree Ramamoorthy
2025-06-20 14:58 ` Andrew Davis [this message]
2025-06-20 15:22   ` Shree Ramamoorthy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67352ec8-6bd0-4434-8553-9fe7829ed188@ti.com \
    --to=afd@ti.com \
    --cc=aaro.koskinen@iki.fi \
    --cc=andreas@kemnade.info \
    --cc=d-gole@ti.com \
    --cc=jkridner@gmail.com \
    --cc=khilman@baylibre.com \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=m-leonard@ti.com \
    --cc=praneeth@ti.com \
    --cc=robertcnelson@gmail.com \
    --cc=rogerq@kernel.org \
    --cc=s-ramamoorthy@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®