Messages in this thread Patch in this message | | | Date | Tue, 4 Jul 2023 21:30:29 +0100 | From | Simon Horman <> | Subject | Re: [PATCH v1] net:thunder_bgx:Fix resource leaks in device_for_each_child_node() loops |
| |
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 -- pw-bot: changes-requested
| |