| Message ID | 20230714100010.12035-1-machel@vivo.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v3] net: thunder: bgx: Fix resource leaks in device_for_each_child_node() loops | expand |
> The device_for_each_child_node() loop in bgx_init_of_phy() > function should have fwnode_handle_put() before break which could > avoid resource leaks. This patch could fix this bug. Are imperative change descriptions still preferred? See also: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc1#n94 Regards, Markus
On Fri, 2023-07-14 at 15:06 +0200, Markus Elfring wrote: > > The device_for_each_child_node() loop in bgx_init_of_phy() > > function should have fwnode_handle_put() before break which could > > avoid resource leaks. This patch could fix this bug. > > Are imperative change descriptions still preferred? Yes. The commit message should be re-phrased. More importantly, it looks like the relevant reference is already released by of_node_put() and the additional fwnode_handle_put() will cause a reference underflow. This patch does not look correct to me. Cheers, Paolo
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c index a317feb8decb..f8a8b2ab72aa 100644 --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c @@ -1469,6 +1469,7 @@ static int bgx_init_of_phy(struct bgx *bgx) struct fwnode_handle *fwn; struct device_node *node = NULL; u8 lmac = 0; + int err = 0; device_for_each_child_node(&bgx->pdev->dev, fwn) { struct phy_device *pd; @@ -1479,7 +1480,7 @@ static int bgx_init_of_phy(struct bgx *bgx) */ node = to_of_node(fwn); if (!node) - break; + goto out_handle_put; of_get_mac_address(node, bgx->lmac[lmac].mac); @@ -1501,10 +1502,8 @@ static int bgx_init_of_phy(struct bgx *bgx) } lmac++; - if (lmac == bgx->max_lmac) { - of_node_put(node); - break; - } + if (lmac == bgx->max_lmac) + goto out_node_put; } return 0; @@ -1519,8 +1518,12 @@ static int bgx_init_of_phy(struct bgx *bgx) } lmac--; } + err = -EPROBE_DEFER; +out_node_put: of_node_put(node); - return -EPROBE_DEFER; +out_handle_put: + fwnode_handle_put(fwn); + return err; } #else
The device_for_each_child_node() loop in bgx_init_of_phy() function should have fwnode_handle_put() before break which could avoid resource leaks. This patch could fix this bug. Fixes: eee326fd8334 ("net: thunderx: bgx: Use standard firmware node infrastructure.") Signed-off-by: Wang Ming <machel@vivo.com> --- drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)