* [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
@ 2020-12-13 11:48 Christophe JAILLET
2020-12-14 11:48 ` Dan Carpenter
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Christophe JAILLET @ 2020-12-13 11:48 UTC (permalink / raw)
To: UNGLinuxDriver, vladimir.oltean, claudiu.manoil,
alexandre.belloni, davem, kuba, andrew
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
In case of error after calling 'ocelot_init()', it must be undone by a
corresponding 'ocelot_deinit()' call, as already done in the remove
function.
Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/net/ethernet/mscc/ocelot_vsc7514.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 1e7729421a82..9cf2bc5f4289 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -1267,7 +1267,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
err = mscc_ocelot_init_ports(pdev, ports);
if (err)
- goto out_put_ports;
+ goto out_ocelot_deinit;
if (ocelot->ptp) {
err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
@@ -1282,8 +1282,14 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
register_switchdev_notifier(&ocelot_switchdev_nb);
register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
+ of_node_put(ports);
+
dev_info(&pdev->dev, "Ocelot switch probed\n");
+ return 0;
+
+out_ocelot_deinit:
+ ocelot_deinit(ocelot);
out_put_ports:
of_node_put(ports);
return err;
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
2020-12-13 11:48 [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function Christophe JAILLET
@ 2020-12-14 11:48 ` Dan Carpenter
2020-12-14 20:00 ` Christophe JAILLET
2020-12-14 22:54 ` Alexandre Belloni
2020-12-16 19:20 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2020-12-14 11:48 UTC (permalink / raw)
To: Christophe JAILLET
Cc: UNGLinuxDriver, vladimir.oltean, claudiu.manoil,
alexandre.belloni, davem, kuba, andrew, netdev, linux-kernel,
kernel-janitors
On Sun, Dec 13, 2020 at 12:48:38PM +0100, Christophe JAILLET wrote:
> In case of error after calling 'ocelot_init()', it must be undone by a
> corresponding 'ocelot_deinit()' call, as already done in the remove
> function.
>
This changes the behavior slightly in another way as well, but it's
probably a bug fix.
drivers/net/ethernet/mscc/ocelot_vsc7514.c
1250 ports = of_get_child_by_name(np, "ethernet-ports");
1251 if (!ports) {
1252 dev_err(ocelot->dev, "no ethernet-ports child node found\n");
1253 return -ENODEV;
1254 }
1255
1256 ocelot->num_phys_ports = of_get_child_count(ports);
1257 ocelot->num_flooding_pgids = 1;
1258
1259 ocelot->vcap = vsc7514_vcap_props;
1260 ocelot->inj_prefix = OCELOT_TAG_PREFIX_NONE;
1261 ocelot->xtr_prefix = OCELOT_TAG_PREFIX_NONE;
1262 ocelot->npi = -1;
1263
1264 err = ocelot_init(ocelot);
1265 if (err)
1266 goto out_put_ports;
1267
1268 err = mscc_ocelot_init_ports(pdev, ports);
1269 if (err)
1270 goto out_put_ports;
1271
1272 if (ocelot->ptp) {
1273 err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
1274 if (err) {
1275 dev_err(ocelot->dev,
1276 "Timestamp initialization failed\n");
1277 ocelot->ptp = 0;
1278 }
In the original code, if ocelot_init_timestamp() failed we returned
a negative error code but now we return success. This probably is what
the original authors intended, though.
1279 }
1280
1281 register_netdevice_notifier(&ocelot_netdevice_nb);
1282 register_switchdev_notifier(&ocelot_switchdev_nb);
1283 register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
1284
1285 dev_info(&pdev->dev, "Ocelot switch probed\n");
1286
1287 out_put_ports:
1288 of_node_put(ports);
1289 return err;
1290 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
2020-12-14 11:48 ` Dan Carpenter
@ 2020-12-14 20:00 ` Christophe JAILLET
2020-12-14 20:00 ` Christophe JAILLET
0 siblings, 1 reply; 6+ messages in thread
From: Christophe JAILLET @ 2020-12-14 20:00 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, kernel-janitors
Le 14/12/2020 à 12:48, Dan Carpenter a écrit :
> On Sun, Dec 13, 2020 at 12:48:38PM +0100, Christophe JAILLET wrote:
>> In case of error after calling 'ocelot_init()', it must be undone by a
>> corresponding 'ocelot_deinit()' call, as already done in the remove
>> function.
>>
>
> This changes the behavior slightly in another way as well, but it's
> probably a bug fix.
>
> drivers/net/ethernet/mscc/ocelot_vsc7514.c
> 1250 ports = of_get_child_by_name(np, "ethernet-ports");
> 1251 if (!ports) {
> 1252 dev_err(ocelot->dev, "no ethernet-ports child node found\n");
> 1253 return -ENODEV;
> 1254 }
> 1255
> 1256 ocelot->num_phys_ports = of_get_child_count(ports);
> 1257 ocelot->num_flooding_pgids = 1;
> 1258
> 1259 ocelot->vcap = vsc7514_vcap_props;
> 1260 ocelot->inj_prefix = OCELOT_TAG_PREFIX_NONE;
> 1261 ocelot->xtr_prefix = OCELOT_TAG_PREFIX_NONE;
> 1262 ocelot->npi = -1;
> 1263
> 1264 err = ocelot_init(ocelot);
> 1265 if (err)
> 1266 goto out_put_ports;
> 1267
> 1268 err = mscc_ocelot_init_ports(pdev, ports);
> 1269 if (err)
> 1270 goto out_put_ports;
> 1271
> 1272 if (ocelot->ptp) {
> 1273 err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
> 1274 if (err) {
> 1275 dev_err(ocelot->dev,
> 1276 "Timestamp initialization failed\n");
> 1277 ocelot->ptp = 0;
> 1278 }
>
> In the original code, if ocelot_init_timestamp() failed we returned
> a negative error code but now we return success. This probably is what
> the original authors intended, though.
>
Thanks for the detailed review Dan.
I agree with you. However this "fix" was not intentional. :(
This may worth stating it in the commit message.
Can it be done when/if the patch is applied?
CJ
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
2020-12-14 20:00 ` Christophe JAILLET
@ 2020-12-14 20:00 ` Christophe JAILLET
0 siblings, 0 replies; 6+ messages in thread
From: Christophe JAILLET @ 2020-12-14 20:00 UTC (permalink / raw)
To: Dan Carpenter
Cc: UNGLinuxDriver, vladimir.oltean, claudiu.manoil,
alexandre.belloni, davem, kuba, andrew, netdev, linux-kernel,
kernel-janitors
Le 14/12/2020 à 12:48, Dan Carpenter a écrit :
> On Sun, Dec 13, 2020 at 12:48:38PM +0100, Christophe JAILLET wrote:
>> In case of error after calling 'ocelot_init()', it must be undone by a
>> corresponding 'ocelot_deinit()' call, as already done in the remove
>> function.
>>
>
> This changes the behavior slightly in another way as well, but it's
> probably a bug fix.
>
> drivers/net/ethernet/mscc/ocelot_vsc7514.c
> 1250 ports = of_get_child_by_name(np, "ethernet-ports");
> 1251 if (!ports) {
> 1252 dev_err(ocelot->dev, "no ethernet-ports child node found\n");
> 1253 return -ENODEV;
> 1254 }
> 1255
> 1256 ocelot->num_phys_ports = of_get_child_count(ports);
> 1257 ocelot->num_flooding_pgids = 1;
> 1258
> 1259 ocelot->vcap = vsc7514_vcap_props;
> 1260 ocelot->inj_prefix = OCELOT_TAG_PREFIX_NONE;
> 1261 ocelot->xtr_prefix = OCELOT_TAG_PREFIX_NONE;
> 1262 ocelot->npi = -1;
> 1263
> 1264 err = ocelot_init(ocelot);
> 1265 if (err)
> 1266 goto out_put_ports;
> 1267
> 1268 err = mscc_ocelot_init_ports(pdev, ports);
> 1269 if (err)
> 1270 goto out_put_ports;
> 1271
> 1272 if (ocelot->ptp) {
> 1273 err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
> 1274 if (err) {
> 1275 dev_err(ocelot->dev,
> 1276 "Timestamp initialization failed\n");
> 1277 ocelot->ptp = 0;
> 1278 }
>
> In the original code, if ocelot_init_timestamp() failed we returned
> a negative error code but now we return success. This probably is what
> the original authors intended, though.
>
Thanks for the detailed review Dan.
I agree with you. However this "fix" was not intentional. :(
This may worth stating it in the commit message.
Can it be done when/if the patch is applied?
CJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
2020-12-13 11:48 [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function Christophe JAILLET
2020-12-14 11:48 ` Dan Carpenter
@ 2020-12-14 22:54 ` Alexandre Belloni
2020-12-16 19:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2020-12-14 22:54 UTC (permalink / raw)
To: Christophe JAILLET
Cc: UNGLinuxDriver, vladimir.oltean, claudiu.manoil, davem, kuba,
andrew, netdev, linux-kernel, kernel-janitors
On 13/12/2020 12:48:38+0100, Christophe JAILLET wrote:
> In case of error after calling 'ocelot_init()', it must be undone by a
> corresponding 'ocelot_deinit()' call, as already done in the remove
> function.
>
> Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> drivers/net/ethernet/mscc/ocelot_vsc7514.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> index 1e7729421a82..9cf2bc5f4289 100644
> --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> @@ -1267,7 +1267,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>
> err = mscc_ocelot_init_ports(pdev, ports);
> if (err)
> - goto out_put_ports;
> + goto out_ocelot_deinit;
>
> if (ocelot->ptp) {
> err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
> @@ -1282,8 +1282,14 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> register_switchdev_notifier(&ocelot_switchdev_nb);
> register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
>
> + of_node_put(ports);
> +
> dev_info(&pdev->dev, "Ocelot switch probed\n");
>
> + return 0;
> +
> +out_ocelot_deinit:
> + ocelot_deinit(ocelot);
> out_put_ports:
> of_node_put(ports);
> return err;
> --
> 2.27.0
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
2020-12-13 11:48 [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function Christophe JAILLET
2020-12-14 11:48 ` Dan Carpenter
2020-12-14 22:54 ` Alexandre Belloni
@ 2020-12-16 19:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-16 19:20 UTC (permalink / raw)
To: Christophe JAILLET
Cc: UNGLinuxDriver, vladimir.oltean, claudiu.manoil,
alexandre.belloni, davem, kuba, andrew, netdev, linux-kernel,
kernel-janitors
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Sun, 13 Dec 2020 12:48:38 +0100 you wrote:
> In case of error after calling 'ocelot_init()', it must be undone by a
> corresponding 'ocelot_deinit()' call, as already done in the remove
> function.
>
> Fixes: a556c76adc05 ("net: mscc: Add initial Ocelot switch support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>
> [...]
Here is the summary with links:
- net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function
https://git.kernel.org/netdev/net/c/f87675b836b3
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-16 19:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-13 11:48 [PATCH] net: mscc: ocelot: Fix a resource leak in the error handling path of the probe function Christophe JAILLET
2020-12-14 11:48 ` Dan Carpenter
2020-12-14 20:00 ` Christophe JAILLET
2020-12-14 20:00 ` Christophe JAILLET
2020-12-14 22:54 ` Alexandre Belloni
2020-12-16 19:20 ` patchwork-bot+netdevbpf
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).