* Re: [PATCH] mfd: sm501: unwind common init failures
2026-06-16 0:41 [PATCH] mfd: sm501: unwind common init failures Pengpeng Hou
@ 2026-07-02 10:57 ` Lee Jones
2026-07-02 11:11 ` Lee Jones
1 sibling, 0 replies; 3+ messages in thread
From: Lee Jones @ 2026-07-02 10:57 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mfd: sm501: unwind common init failures
2026-06-16 0:41 [PATCH] mfd: sm501: unwind common init failures Pengpeng Hou
2026-07-02 10:57 ` Lee Jones
@ 2026-07-02 11:11 ` Lee Jones
1 sibling, 0 replies; 3+ messages in thread
From: Lee Jones @ 2026-07-02 11:11 UTC (permalink / raw)
To: Pengpeng Hou, Ben Dooks; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread