* [PATCH] mfd: tps6594: copy regmap IRQ chip descriptors per probe
@ 2026-06-11 14:56 Runyu Xiao
2026-06-18 10:40 ` Lee Jones
0 siblings, 1 reply; 2+ messages in thread
From: Runyu Xiao @ 2026-06-11 14:56 UTC (permalink / raw)
To: lee; +Cc: jpanis, bhargav.r, mwalle, linux-kernel, jianhao.xu, runyu.xiao,
stable
tps6594_device_init() selects one of several shared static
struct regmap_irq_chip templates and then writes the current probe's
irq_drv_data and generated name into that shared descriptor before
passing it to devm_regmap_add_irq_chip().
On a running system this is reachable whenever another TPS6594,
TPS65224, or TPS652G1 instance probes through the same descriptor
family. regmap-irq keeps the raw chip pointer, so the later probe
overwrites the earlier instance's callback context. A later IRQ can
then run tps6594_handle_post_irq() with the wrong struct tps6594,
name, chip_id, regmap, and CRC handling path.
The issue was found on Linux v6.18.21 during manual auditing of drivers
that reuse shared regmap_irq_chip descriptors while filling probe-local
irq_drv_data and name fields before devm_regmap_add_irq_chip(), and was
confirmed with a focused QEMU no-device validation harness. That test
showed a later probe could overwrite the earlier registration's saved
callback context through the shared chip descriptor, while per-probe
descriptor copies preserved callback ownership for both registrations.
Copy the selected descriptor with devm_kmemdup(), mutate only the
copy, and pass that copy to devm_regmap_add_irq_chip(). Also mark the
static descriptors const so probe-local state cannot be written back
into shared templates again.
Fixes: 325bec7157b3 ("mfd: tps6594: Add driver for TI TPS6594 PMIC")
Fixes: 9d855b8144e6 ("mfd: tps6594-core: Add TI TPS65224 PMIC core")
Fixes: 626bb0a45584 ("mfd: tps6594: Add TI TPS652G1 support")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/mfd/tps6594-core.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/tps6594-core.c b/drivers/mfd/tps6594-core.c
index 8b26c4127472..36904979b6b0 100644
--- a/drivers/mfd/tps6594-core.c
+++ b/drivers/mfd/tps6594-core.c
@@ -531,7 +531,7 @@ static int tps6594_handle_post_irq(void *irq_drv_data)
return ret;
};
-static struct regmap_irq_chip tps6594_irq_chip = {
+static const struct regmap_irq_chip tps6594_irq_chip = {
.ack_base = TPS6594_REG_INT_BUCK1_2,
.ack_invert = 1,
.clear_ack = 1,
@@ -543,7 +543,7 @@ static struct regmap_irq_chip tps6594_irq_chip = {
.handle_post_irq = tps6594_handle_post_irq,
};
-static struct regmap_irq_chip tps65224_irq_chip = {
+static const struct regmap_irq_chip tps65224_irq_chip = {
.ack_base = TPS6594_REG_INT_BUCK,
.ack_invert = 1,
.clear_ack = 1,
@@ -555,7 +555,7 @@ static struct regmap_irq_chip tps65224_irq_chip = {
.handle_post_irq = tps6594_handle_post_irq,
};
-static struct regmap_irq_chip tps652g1_irq_chip = {
+static const struct regmap_irq_chip tps652g1_irq_chip = {
.ack_base = TPS6594_REG_INT_BUCK,
.ack_invert = 1,
.clear_ack = 1,
@@ -707,7 +707,10 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
{
struct device *dev = tps->dev;
int ret;
- struct regmap_irq_chip *irq_chip;
+ const struct regmap_irq_chip *irq_chip;
+ struct regmap_irq_chip irq_chip_copy;
+ const char *irq_chip_name;
+ void *irq_chip_desc;
unsigned int pwr_on, gpio3_cfg;
const struct mfd_cell *cells;
int n_cells;
@@ -738,15 +741,22 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
cells = tps6594_common_cells;
}
- irq_chip->irq_drv_data = tps;
- irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
- dev->driver->name, tps->chip_id, tps->reg);
+ irq_chip_name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
+ dev->driver->name, tps->chip_id, tps->reg);
+ if (!irq_chip_name)
+ return -ENOMEM;
+
+ irq_chip_copy = *irq_chip;
+ irq_chip_copy.irq_drv_data = tps;
+ irq_chip_copy.name = irq_chip_name;
- if (!irq_chip->name)
+ irq_chip_desc = devm_kmemdup(dev, &irq_chip_copy, sizeof(irq_chip_copy),
+ GFP_KERNEL);
+ if (!irq_chip_desc)
return -ENOMEM;
ret = devm_regmap_add_irq_chip(dev, tps->regmap, tps->irq, IRQF_SHARED | IRQF_ONESHOT,
- 0, irq_chip, &tps->irq_data);
+ 0, irq_chip_desc, &tps->irq_data);
if (ret)
return dev_err_probe(dev, ret, "Failed to add regmap IRQ\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] mfd: tps6594: copy regmap IRQ chip descriptors per probe
2026-06-11 14:56 [PATCH] mfd: tps6594: copy regmap IRQ chip descriptors per probe Runyu Xiao
@ 2026-06-18 10:40 ` Lee Jones
0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-06-18 10:40 UTC (permalink / raw)
To: Runyu Xiao; +Cc: jpanis, bhargav.r, mwalle, linux-kernel, jianhao.xu, stable
On Thu, 11 Jun 2026, Runyu Xiao wrote:
> tps6594_device_init() selects one of several shared static
> struct regmap_irq_chip templates and then writes the current probe's
> irq_drv_data and generated name into that shared descriptor before
> passing it to devm_regmap_add_irq_chip().
>
> On a running system this is reachable whenever another TPS6594,
> TPS65224, or TPS652G1 instance probes through the same descriptor
> family. regmap-irq keeps the raw chip pointer, so the later probe
> overwrites the earlier instance's callback context. A later IRQ can
> then run tps6594_handle_post_irq() with the wrong struct tps6594,
> name, chip_id, regmap, and CRC handling path.
>
> The issue was found on Linux v6.18.21 during manual auditing of drivers
> that reuse shared regmap_irq_chip descriptors while filling probe-local
> irq_drv_data and name fields before devm_regmap_add_irq_chip(), and was
> confirmed with a focused QEMU no-device validation harness. That test
> showed a later probe could overwrite the earlier registration's saved
> callback context through the shared chip descriptor, while per-probe
> descriptor copies preserved callback ownership for both registrations.
>
> Copy the selected descriptor with devm_kmemdup(), mutate only the
> copy, and pass that copy to devm_regmap_add_irq_chip(). Also mark the
> static descriptors const so probe-local state cannot be written back
> into shared templates again.
>
> Fixes: 325bec7157b3 ("mfd: tps6594: Add driver for TI TPS6594 PMIC")
> Fixes: 9d855b8144e6 ("mfd: tps6594-core: Add TI TPS65224 PMIC core")
> Fixes: 626bb0a45584 ("mfd: tps6594: Add TI TPS652G1 support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> drivers/mfd/tps6594-core.c | 28 +++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git "a/drivers/mfd/tps6594-core.c" "b/drivers/mfd/tps6594-core.c"
> index 8b26c4127472..36904979b6b0 100644
> --- "a/drivers/mfd/tps6594-core.c"
> +++ "b/drivers/mfd/tps6594-core.c"
> @@ -531,7 +531,7 @@ static int tps6594_handle_post_irq(void *irq_drv_data)
> return ret;
> };
>
> -static struct regmap_irq_chip tps6594_irq_chip = {
> +static const struct regmap_irq_chip tps6594_irq_chip = {
> .ack_base = TPS6594_REG_INT_BUCK1_2,
> .ack_invert = 1,
> .clear_ack = 1,
> @@ -543,7 +543,7 @@ static struct regmap_irq_chip tps6594_irq_chip = {
> .handle_post_irq = tps6594_handle_post_irq,
> };
>
> -static struct regmap_irq_chip tps65224_irq_chip = {
> +static const struct regmap_irq_chip tps65224_irq_chip = {
> .ack_base = TPS6594_REG_INT_BUCK,
> .ack_invert = 1,
> .clear_ack = 1,
> @@ -555,7 +555,7 @@ static struct regmap_irq_chip tps65224_irq_chip = {
> .handle_post_irq = tps6594_handle_post_irq,
> };
>
> -static struct regmap_irq_chip tps652g1_irq_chip = {
> +static const struct regmap_irq_chip tps652g1_irq_chip = {
> .ack_base = TPS6594_REG_INT_BUCK,
> .ack_invert = 1,
> .clear_ack = 1,
> @@ -707,7 +707,10 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
> {
> struct device *dev = tps->dev;
> int ret;
> - struct regmap_irq_chip *irq_chip;
> + const struct regmap_irq_chip *irq_chip;
> + struct regmap_irq_chip irq_chip_copy;
> + const char *irq_chip_name;
> + void *irq_chip_desc;
Putting irq_chip_copy on the stack and using void* here is pretty rough.
How about declaring a typed 'struct regmap_irq_chip *chip' pointer
instead would keep things cleaner.
> unsigned int pwr_on, gpio3_cfg;
> const struct mfd_cell *cells;
> int n_cells;
> @@ -738,15 +741,22 @@ int tps6594_device_init(struct tps6594 *tps, bool enable_crc)
> cells = tps6594_common_cells;
> }
>
> - irq_chip->irq_drv_data = tps;
> - irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
> - dev->driver->name, tps->chip_id, tps->reg);
> + irq_chip_name = devm_kasprintf(dev, GFP_KERNEL, "%s-%ld-0x%02x",
> + dev->driver->name, tps->chip_id, tps->reg);
> + if (!irq_chip_name)
> + return -ENOMEM;
> +
> + irq_chip_copy = *irq_chip;
> + irq_chip_copy.irq_drv_data = tps;
> + irq_chip_copy.name = irq_chip_name;
>
> - if (!irq_chip->name)
> + irq_chip_desc = devm_kmemdup(dev, &irq_chip_copy, sizeof(irq_chip_copy),
> + GFP_KERNEL);
Then we can perform the 'devm_kmemdup()' call first using the template
pointer and modify the heap-allocated structure directly.
How about:
chip = devm_kmemdup(dev, irq_chip, sizeof(*chip), GFP_KERNEL);
if (!chip)
return -ENOMEM;
chip->irq_drv_data = tps;
chip->name = irq_chip_name;
> + if (!irq_chip_desc)
> return -ENOMEM;
>
> ret = devm_regmap_add_irq_chip(dev, tps->regmap, tps->irq, IRQF_SHARED | IRQF_ONESHOT,
> - 0, irq_chip, &tps->irq_data);
> + 0, irq_chip_desc, &tps->irq_data);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to add regmap IRQ\n");
>
> --
> 2.34.1
--
Lee Jones
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-06-18 10:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-11 14:56 [PATCH] mfd: tps6594: copy regmap IRQ chip descriptors per probe Runyu Xiao
2026-06-18 10:40 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox