[PATCH v4 phy-next 1/9] soc: fsl: guts: perform fsl_guts_init() error teardown in reverse order of setup
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: 2026-07-21 23:16:38
Also in:
linux-arm-kernel, linux-devicetree, linux-phy, lkml
Subsystem:
freescale soc drivers, the rest · Maintainers:
Christophe Leroy, Linus Torvalds
fsl_guts_init() is about to get much more complicated and the central error handling procedure cannot scale in its current design, unless we add a lot of "if" conditions to detect what has been allocated and what hasn't. Currently the code relies on the fact that kfree(NULL) is safe, but this doesn't scale to the case where "soc_dev_attr" itself is NULL, because this would dereference "soc_dev_attr->family" and friends of a NULL pointer. Convert to the more typical error handling pattern where the teardown is in the strict reverse order of setup, and a teardown step is only called if its corresponding setup step was executed. At the same time, maintain the optionality of soc_dev_attr->serial_number by not checking whether that kasprintf() has returned NULL. In the error path, kfree(NULL) is safe, so we don't need to add an "if" condition for it. Michael Walle has confirmed that ignoring the error was intentional, and we preserve that: https://lore.kernel.org/linux-phy/DK44809N7Y8I.J2Z3U4N32H0Q@kernel.org/ (local) Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- v3->v4: patch is new --- drivers/soc/fsl/guts.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 9bee7baec2b9..453456f31800 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c@@ -227,17 +227,23 @@ static int __init fsl_guts_init(void) } else { soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ"); } - if (!soc_dev_attr->family) - goto err_nomem; + if (!soc_dev_attr->family) { + ret = -ENOMEM; + goto err_free_soc_dev_attr; + } soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "svr:0x%08x", svr); - if (!soc_dev_attr->soc_id) - goto err_nomem; + if (!soc_dev_attr->soc_id) { + ret = -ENOMEM; + goto err_free_family; + } soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d.%d", (svr >> 4) & 0xf, svr & 0xf); - if (!soc_dev_attr->revision) - goto err_nomem; + if (!soc_dev_attr->revision) { + ret = -ENOMEM; + goto err_free_soc_id; + } if (soc_data) soc_uid = fsl_guts_get_soc_uid(soc_data->sfp_compat,
@@ -249,7 +255,7 @@ static int __init fsl_guts_init(void) soc_dev = soc_device_register(soc_dev_attr); if (IS_ERR(soc_dev)) { ret = PTR_ERR(soc_dev); - goto err; + goto err_free_serial_number; } pr_info("Machine: %s\n", soc_dev_attr->machine);
@@ -259,13 +265,14 @@ static int __init fsl_guts_init(void) return 0; -err_nomem: - ret = -ENOMEM; -err: - kfree(soc_dev_attr->family); - kfree(soc_dev_attr->soc_id); - kfree(soc_dev_attr->revision); +err_free_serial_number: kfree(soc_dev_attr->serial_number); + kfree(soc_dev_attr->revision); +err_free_soc_id: + kfree(soc_dev_attr->soc_id); +err_free_family: + kfree(soc_dev_attr->family); +err_free_soc_dev_attr: kfree(soc_dev_attr); return ret;
--
2.34.1