[PATCH] mfd: sm501: unwind common init failures

[PATCH] mfd: sm501: unwind common init failures

[Pengpeng Hou]

sm501_init_dev() creates the debug sysfs file and can register child
devices before the final clock-source sanity check. If that check fails,
the caller tears down the parent mapping but the children and debug file
remain published. The PCI probe path also ignored the return value from
sm501_init_dev().

Unwind the common SM501 registrations before returning the clock-check
failure, and make the PCI probe path propagate sm501_init_dev() errors.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/mfd/sm501.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
index 0ee6d8940e69..b21b4fc18e87 100644
--- a/drivers/mfd/sm501.c
+++ b/drivers/mfd/sm501.c
@@ -1249,6 +1249,8 @@ static unsigned int sm501_mem_local[] = {
 	[5]	= 2*1024*1024,
 };
 
+static void sm501_dev_remove(struct sm501_devdata *sm);
+
 /* sm501_init_dev
  *
  * Common init code for an SM501
@@ -1321,6 +1323,7 @@ static int sm501_init_dev(struct sm501_devdata *sm)
 	if (ret) {
 		dev_err(sm->dev, "M1X and M clocks sourced from different "
 					"PLLs\n");
+		sm501_dev_remove(sm);
 		return -EINVAL;
 	}
 
@@ -1583,9 +1586,14 @@ static int sm501_pci_probe(struct pci_dev *dev,
 		goto err4;
 	}
 
-	sm501_init_dev(sm);
+	err = sm501_init_dev(sm);
+	if (err)
+		goto err5;
+
 	return 0;
 
+ err5:
+	iounmap(sm->regs);
  err4:
 	release_mem_region(sm->io_res->start, 0x100);
  err3:
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] mfd: sm501: unwind common init failures

[Lee Jones]

Can you consider these please.

Also, we don't allow forward declarations.

/* Sashiko Automation: Reviewed (2 Findings) */

On Tue, 16 Jun 2026, Pengpeng Hou wrote:

> sm501_init_dev() creates the debug sysfs file and can register child
> devices before the final clock-source sanity check. If that check fails,
> the caller tears down the parent mapping but the children and debug file
> remain published. The PCI probe path also ignored the return value from
> sm501_init_dev().
> 
> Unwind the common SM501 registrations before returning the clock-check
> failure, and make the PCI probe path propagate sm501_init_dev() errors.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/mfd/sm501.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
> index 0ee6d8940e69..b21b4fc18e87 100644
> --- a/drivers/mfd/sm501.c
> +++ b/drivers/mfd/sm501.c
> @@ -1249,6 +1249,8 @@ static unsigned int sm501_mem_local[] = {
>  	[5]	= 2*1024*1024,
>  };
>  
> +static void sm501_dev_remove(struct sm501_devdata *sm);
> +
>  /* sm501_init_dev
>   *
>   * Common init code for an SM501
> @@ -1321,6 +1323,7 @@ static int sm501_init_dev(struct sm501_devdata *sm)
>  	if (ret) {
>  		dev_err(sm->dev, "M1X and M clocks sourced from different "
>  					"PLLs\n");
> +		sm501_dev_remove(sm);

[Severity: High]
This isn't a bug introduced by this patch, but can this error path unwinding
lead to a Use-After-Free?

When sm501_dev_remove() unregisters the child platform devices in
sm501_remove_sub(), the devres framework automatically frees the memory
allocated for the GPIO lookup table.

However, since gpiod_remove_lookup_table() is never called to remove it
from the global gpio_lookup_list, the freed memory remains linked. Will any
subsequent GPIO lookup system-wide traverse this corrupted list, resulting
in a Use-After-Free?



[Severity: High]
This is another pre-existing issue, but does calling devm_kzalloc() on an
uninitialized platform device trigger a kernel panic here?

Earlier in initialization, sm501_register_gpio_i2c_instance() allocates the
platform device using kzalloc() and then immediately calls devm_kzalloc()
to allocate the GPIO lookup table:

drivers/mfd/sm501.c:sm501_register_gpio_i2c_instance() {
    ...
    pdev = sm501_create_subdev(sm, "i2c-gpio", 0,
                   sizeof(struct i2c_gpio_platform_data));
    ...
    lookup = devm_kzalloc(&pdev->dev, struct_size(lookup, table, 3),
                  GFP_KERNEL);
    ...
}

Because device_initialize() hasn't been called on pdev yet,
pdev->dev.devres_head is completely uninitialized. When devres attempts to
attach the node, wouldn't it call list_add_tail() on an uninitialized list
head and dereference a NULL pointer?


>  		return -EINVAL;
>  	}
>  
> @@ -1583,9 +1586,14 @@ static int sm501_pci_probe(struct pci_dev *dev,
>  		goto err4;
>  	}
>  
> -	sm501_init_dev(sm);
> +	err = sm501_init_dev(sm);
> +	if (err)
> +		goto err5;
> +
>  	return 0;
>  
> + err5:
> +	iounmap(sm->regs);
>   err4:
>  	release_mem_region(sm->io_res->start, 0x100);
>   err3:
> -- 
> 2.50.1 (Apple Git-155)
> 

-- 
Lee Jones

Re: [PATCH] mfd: sm501: unwind common init failures

[Lee Jones]

Once again, you're missing with ancient code.

You need to draft in the author.

On Tue, 16 Jun 2026, Pengpeng Hou wrote:
> sm501_init_dev() creates the debug sysfs file and can register child
> devices before the final clock-source sanity check. If that check fails,
> the caller tears down the parent mapping but the children and debug file
> remain published. The PCI probe path also ignored the return value from
> sm501_init_dev().
> 
> Unwind the common SM501 registrations before returning the clock-check
> failure, and make the PCI probe path propagate sm501_init_dev() errors.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/mfd/sm501.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/sm501.c b/drivers/mfd/sm501.c
> index 0ee6d8940e69..b21b4fc18e87 100644
> --- a/drivers/mfd/sm501.c
> +++ b/drivers/mfd/sm501.c
> @@ -1249,6 +1249,8 @@ static unsigned int sm501_mem_local[] = {
>  	[5]	= 2*1024*1024,
>  };
>  
> +static void sm501_dev_remove(struct sm501_devdata *sm);
> +
>  /* sm501_init_dev
>   *
>   * Common init code for an SM501
> @@ -1321,6 +1323,7 @@ static int sm501_init_dev(struct sm501_devdata *sm)
>  	if (ret) {
>  		dev_err(sm->dev, "M1X and M clocks sourced from different "
>  					"PLLs\n");
> +		sm501_dev_remove(sm);
>  		return -EINVAL;
>  	}
>  
> @@ -1583,9 +1586,14 @@ static int sm501_pci_probe(struct pci_dev *dev,
>  		goto err4;
>  	}
>  
> -	sm501_init_dev(sm);
> +	err = sm501_init_dev(sm);
> +	if (err)
> +		goto err5;
> +
>  	return 0;
>  
> + err5:
> +	iounmap(sm->regs);
>   err4:
>  	release_mem_region(sm->io_res->start, 0x100);
>   err3:
> -- 
> 2.50.1 (Apple Git-155)
> 

-- 
Lee Jones