Skip to content

Commit 29afbd7

Browse files
Dan Carpentervinodkoul
authored andcommitted
phy: cadence: Sierra: fix error handling bugs in probe()
There are two bugs in the error handling: 1: If devm_of_phy_provider_register() fails then there was no cleanup. 2: The error handling called of_node_put(child) improperly leading to a use after free. We are only holding the reference inside the loop so the last two gotos after the loop lead to a use after free bug. Fix this by cleaning up the partial allocations (or partial iterations) in the loop before doing the goto. Fixes: a43f72a ("phy: cadence: Sierra: Change MAX_LANES of Sierra to 16") Fixes: 44d30d6 ("phy: cadence: Add driver for Sierra PHY") Signed-off-by: Dan Carpenter <[email protected]> Link: https://lore.kernel.org/r/20220115115146.GC7552@kili Signed-off-by: Vinod Koul <[email protected]>
1 parent 6d1e6bc commit 29afbd7

File tree

1 file changed

+21
-14
lines changed

1 file changed

+21
-14
lines changed

drivers/phy/cadence/phy-cadence-sierra.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,7 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
13381338
struct device *dev = &pdev->dev;
13391339
const struct cdns_sierra_data *data;
13401340
unsigned int id_value;
1341-
int i, ret, node = 0;
1341+
int ret, node = 0;
13421342
void __iomem *base;
13431343
struct device_node *dn = dev->of_node, *child;
13441344

@@ -1416,15 +1416,18 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
14161416
dev_err(dev, "failed to get reset %s\n",
14171417
child->full_name);
14181418
ret = PTR_ERR(sp->phys[node].lnk_rst);
1419-
goto put_child2;
1419+
of_node_put(child);
1420+
goto put_control;
14201421
}
14211422

14221423
if (!sp->autoconf) {
14231424
ret = cdns_sierra_get_optional(&sp->phys[node], child);
14241425
if (ret) {
14251426
dev_err(dev, "missing property in node %s\n",
14261427
child->name);
1427-
goto put_child;
1428+
of_node_put(child);
1429+
reset_control_put(sp->phys[node].lnk_rst);
1430+
goto put_control;
14281431
}
14291432
}
14301433

@@ -1434,7 +1437,9 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
14341437

14351438
if (IS_ERR(gphy)) {
14361439
ret = PTR_ERR(gphy);
1437-
goto put_child;
1440+
of_node_put(child);
1441+
reset_control_put(sp->phys[node].lnk_rst);
1442+
goto put_control;
14381443
}
14391444
sp->phys[node].phy = gphy;
14401445
phy_set_drvdata(gphy, &sp->phys[node]);
@@ -1446,26 +1451,28 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
14461451
if (sp->num_lanes > SIERRA_MAX_LANES) {
14471452
ret = -EINVAL;
14481453
dev_err(dev, "Invalid lane configuration\n");
1449-
goto put_child2;
1454+
goto put_control;
14501455
}
14511456

14521457
/* If more than one subnode, configure the PHY as multilink */
14531458
if (!sp->autoconf && sp->nsubnodes > 1) {
14541459
ret = cdns_sierra_phy_configure_multilink(sp);
14551460
if (ret)
1456-
goto put_child2;
1461+
goto put_control;
14571462
}
14581463

14591464
pm_runtime_enable(dev);
14601465
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
1461-
return PTR_ERR_OR_ZERO(phy_provider);
1462-
1463-
put_child:
1464-
node++;
1465-
put_child2:
1466-
for (i = 0; i < node; i++)
1467-
reset_control_put(sp->phys[i].lnk_rst);
1468-
of_node_put(child);
1466+
if (IS_ERR(phy_provider)) {
1467+
ret = PTR_ERR(phy_provider);
1468+
goto put_control;
1469+
}
1470+
1471+
return 0;
1472+
1473+
put_control:
1474+
while (--node >= 0)
1475+
reset_control_put(sp->phys[node].lnk_rst);
14691476
clk_disable:
14701477
cdns_sierra_phy_disable_clocks(sp);
14711478
reset_control_assert(sp->apb_rst);

0 commit comments

Comments
 (0)