From: Dan Carpenter <dan.carpenter@oracle.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
UNGLinuxDriver@microchip.com,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: [PATCH 1/2 net-next] net: mscc: ocelot: fix error handling bugs in mscc_ocelot_init_ports()
Date: Mon, 25 Jan 2021 11:12:42 +0300 [thread overview]
Message-ID: <YA59en4lJCiYsPHv@mwanda> (raw)
There are several error handling bugs in mscc_ocelot_init_ports(). I
went through the code, and carefully audited it and made fixes and
cleanups.
1) The ocelot_probe_port() function didn't have a mirror release function
so it was hard to follow. I created the ocelot_release_port()
function.
2) In the ocelot_probe_port() function, if the register_netdev() call
failed, then it lead to a double free_netdev(dev) bug. Fix this
by moving the "ocelot->ports[port] = ocelot_port;" assignment to the
end of the function after everything has succeeded.
3) I was concerned that the "port" which comes from of_property_read_u32()
might be out of bounds so I added a check for that.
4) In the original code if ocelot_regmap_init() failed then the driver
tried to continue but I think that should be a fatal error.
5) If ocelot_probe_port() failed then the most recent devlink was leaked.
Fix this by moving the "registered_ports[port] = true;" assignment
earlier.
6) The error handling if the final ocelot_port_devlink_init() failed had
two problems. The "while (port-- >= 0)" loop should have been
"--port" pre-op instead of a post-op to avoid a buffer underflow.
The "if (!registered_ports[port])" condition was reversed leading to
resource leaks and double frees.
Fixes: 6c30384eb1de ("net: mscc: ocelot: register devlink ports")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 39 +++++++++-------------
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 30a38df08a21..2c82ffe2c611 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -1064,7 +1064,6 @@ static void mscc_ocelot_release_ports(struct ocelot *ocelot)
int port;
for (port = 0; port < ocelot->num_phys_ports; port++) {
- struct ocelot_port_private *priv;
struct ocelot_port *ocelot_port;
ocelot_port = ocelot->ports[port];
@@ -1072,12 +1071,7 @@ static void mscc_ocelot_release_ports(struct ocelot *ocelot)
continue;
ocelot_deinit_port(ocelot, port);
-
- priv = container_of(ocelot_port, struct ocelot_port_private,
- port);
-
- unregister_netdev(priv->dev);
- free_netdev(priv->dev);
+ ocelot_release_port(ocelot_port);
}
}
@@ -1123,14 +1117,22 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
continue;
port = reg;
+ if (port < 0 || port >= ocelot->num_phys_ports) {
+ dev_err(ocelot->dev,
+ "invalid port number: %d >= %d\n", port,
+ ocelot->num_phys_ports);
+ continue;
+ }
snprintf(res_name, sizeof(res_name), "port%d", port);
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
res_name);
target = ocelot_regmap_init(ocelot, res);
- if (IS_ERR(target))
- continue;
+ if (IS_ERR(target)) {
+ err = PTR_ERR(target);
+ goto out_teardown;
+ }
phy_node = of_parse_phandle(portnp, "phy-handle", 0);
if (!phy_node)
@@ -1147,6 +1149,7 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
of_node_put(portnp);
goto out_teardown;
}
+ registered_ports[port] = true;
err = ocelot_probe_port(ocelot, port, target, phy);
if (err) {
@@ -1154,8 +1157,6 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
goto out_teardown;
}
- registered_ports[port] = true;
-
ocelot_port = ocelot->ports[port];
priv = container_of(ocelot_port, struct ocelot_port_private,
port);
@@ -1213,15 +1214,9 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
err = ocelot_port_devlink_init(ocelot, port,
DEVLINK_PORT_FLAVOUR_UNUSED);
- if (err) {
- while (port-- >= 0) {
- if (!registered_ports[port])
- continue;
- ocelot_port_devlink_teardown(ocelot, port);
- }
-
+ if (err)
goto out_teardown;
- }
+ registered_ports[port] = true;
}
kfree(registered_ports);
@@ -1233,10 +1228,8 @@ static int mscc_ocelot_init_ports(struct platform_device *pdev,
mscc_ocelot_release_ports(ocelot);
/* Tear down devlink ports for the registered network interfaces */
for (port = 0; port < ocelot->num_phys_ports; port++) {
- if (!registered_ports[port])
- continue;
-
- ocelot_port_devlink_teardown(ocelot, port);
+ if (registered_ports[port])
+ ocelot_port_devlink_teardown(ocelot, port);
}
kfree(registered_ports);
return err;
--
2.29.2
next reply other threads:[~2021-01-26 4:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-25 8:12 Dan Carpenter [this message]
2021-01-25 8:13 ` [PATCH 2/2 net-next] net: mscc: ocelot: fix error code in mscc_ocelot_probe() Dan Carpenter
2021-01-25 13:23 ` Vladimir Oltean
2021-01-25 8:19 ` [PATCH 1/2 net-next] net: mscc: ocelot: fix error handling bugs in mscc_ocelot_init_ports() Dan Carpenter
2021-01-25 8:42 ` [PATCH v2 " Dan Carpenter
2021-01-25 16:18 ` Vladimir Oltean
2021-01-26 7:27 ` Dan Carpenter
2021-01-26 8:51 ` Vladimir Oltean
2021-01-25 8:42 ` [PATCH v2 2/2 net-next] net: mscc: ocelot: fix error code in mscc_ocelot_probe() Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YA59en4lJCiYsPHv@mwanda \
--to=dan.carpenter@oracle.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=kernel-janitors@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).