Skip to content

Commit

Permalink
phy: cadence: Sierra: fix error handling bugs in probe()
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
Dan Carpenter authored and vinodkoul committed Jan 24, 2022
1 parent 6d1e6bc commit 29afbd7
Showing 1 changed file with 21 additions and 14 deletions.
35 changes: 21 additions & 14 deletions drivers/phy/cadence/phy-cadence-sierra.c
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
const struct cdns_sierra_data *data;
unsigned int id_value;
int i, ret, node = 0;
int ret, node = 0;
void __iomem *base;
struct device_node *dn = dev->of_node, *child;

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

if (!sp->autoconf) {
ret = cdns_sierra_get_optional(&sp->phys[node], child);
if (ret) {
dev_err(dev, "missing property in node %s\n",
child->name);
goto put_child;
of_node_put(child);
reset_control_put(sp->phys[node].lnk_rst);
goto put_control;
}
}

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

if (IS_ERR(gphy)) {
ret = PTR_ERR(gphy);
goto put_child;
of_node_put(child);
reset_control_put(sp->phys[node].lnk_rst);
goto put_control;
}
sp->phys[node].phy = gphy;
phy_set_drvdata(gphy, &sp->phys[node]);
Expand All @@ -1446,26 +1451,28 @@ static int cdns_sierra_phy_probe(struct platform_device *pdev)
if (sp->num_lanes > SIERRA_MAX_LANES) {
ret = -EINVAL;
dev_err(dev, "Invalid lane configuration\n");
goto put_child2;
goto put_control;
}

/* If more than one subnode, configure the PHY as multilink */
if (!sp->autoconf && sp->nsubnodes > 1) {
ret = cdns_sierra_phy_configure_multilink(sp);
if (ret)
goto put_child2;
goto put_control;
}

pm_runtime_enable(dev);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);

put_child:
node++;
put_child2:
for (i = 0; i < node; i++)
reset_control_put(sp->phys[i].lnk_rst);
of_node_put(child);
if (IS_ERR(phy_provider)) {
ret = PTR_ERR(phy_provider);
goto put_control;
}

return 0;

put_control:
while (--node >= 0)
reset_control_put(sp->phys[node].lnk_rst);
clk_disable:
cdns_sierra_phy_disable_clocks(sp);
reset_control_assert(sp->apb_rst);
Expand Down

0 comments on commit 29afbd7

Please sign in to comment.