From d2fb943cba02142e92236ae7250193af40bae077 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 22 Jul 2026 02:15:55 +0300 Subject: [PATCH] soc: fsl: guts: perform fsl_guts_init() error teardown in reverse order of setup 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/ Signed-off-by: Vladimir Oltean Reviewed-by: Ioana Ciornei Link: https://lore.kernel.org/r/20260721231603.67865-2-vladimir.oltean@nxp.com Signed-off-by: Christophe Leroy (CS GROUP) --- 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;