| Message ID | 20230704141457.4844-1-machel@vivo.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1] net:thunder_bgx:Fix resource leaks in device_for_each_child_node() loops | expand |
On Tue, Jul 04, 2023 at 10:14:47PM +0800, Wang Ming 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. Hi Wang Ming, If this fixes a bug then it probably warrants a fixes tag. And it should also probably be targeted at the next tree: [PATCH net v2] ... If so, please allow 24h to elapse before posting v2. Else it should be targeted at the net-next tree, and posted after net-next reopens after July 10th: [PATCH net-next v2] ... Also, looking at git history, I think a more appropriate subject prefix would be 'net: thunderx: ' [PATCH net v2] net: thunderx: ... Link: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html > Signed-off-by: Wang Ming <machel@vivo.com> > --- > drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > index a317feb8d..dad32d36a 100644 > --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > @@ -1478,8 +1478,10 @@ static int bgx_init_of_phy(struct bgx *bgx) > * cannot handle it, so exit the loop. > */ > node = to_of_node(fwn); > - if (!node) > + if (!node) { > + fwnode_handle_put(fwn); > break; > + } > > of_get_mac_address(node, bgx->lmac[lmac].mac); > > @@ -1503,6 +1505,7 @@ static int bgx_init_of_phy(struct bgx *bgx) > lmac++; > if (lmac == bgx->max_lmac) { > of_node_put(node); > + fwnode_handle_put(fwn); > break; > } > } Should this change also apply to the 'goto defer' case in the same loop? If so, perhaps a more idiomatic approach of releasing resources in a ladder of goto labels is appropriate. Something like this (untested!): diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c index a317feb8decb..3091c96134e4 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
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c index a317feb8d..dad32d36a 100644 --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c @@ -1478,8 +1478,10 @@ static int bgx_init_of_phy(struct bgx *bgx) * cannot handle it, so exit the loop. */ node = to_of_node(fwn); - if (!node) + if (!node) { + fwnode_handle_put(fwn); break; + } of_get_mac_address(node, bgx->lmac[lmac].mac); @@ -1503,6 +1505,7 @@ static int bgx_init_of_phy(struct bgx *bgx) lmac++; if (lmac == bgx->max_lmac) { of_node_put(node); + fwnode_handle_put(fwn); break; } }
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. Signed-off-by: Wang Ming <machel@vivo.com> --- drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)