| Message ID | 20190925182405.31287-1-navid.emamdoost@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | nfp: flower: prevent memory leak in nfp_flower_spawn_phy_reprs | expand |
On Wed, 25 Sep 2019 13:24:02 -0500, Navid Emamdoost wrote: > In nfp_flower_spawn_phy_reprs, in the for loop over eth_tbl if any of > intermediate allocations or initializations fail memory is leaked. > requiered releases are added. > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> Fixes: b94524529741 ("nfp: flower: add per repr private data for LAG offload") Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> In nfp_flower_spawn_phy_reprs, in the for loop over eth_tbl if any of > intermediate allocations or initializations fail memory is leaked. > requiered releases are added. I suggest to improve also this change description. > @@ -542,6 +545,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv) > err = nfp_repr_init(app, repr, > cmsg_port_id, port, priv->nn->dp.netdev); > if (err) { > + kfree(repr_priv); > nfp_port_free(port); > nfp_repr_free(repr); > goto err_reprs_clean; How do you think about to move common exception handling code to the end of this function implementation by adding jump targets? Regards, Markus
From: Navid Emamdoost <navid.emamdoost@gmail.com> Date: Wed, 25 Sep 2019 13:24:02 -0500 > In nfp_flower_spawn_phy_reprs, in the for loop over eth_tbl if any of > intermediate allocations or initializations fail memory is leaked. > requiered releases are added. > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> Applied.
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c index 7a20447cca19..91a47899220f 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/main.c +++ b/drivers/net/ethernet/netronome/nfp/flower/main.c @@ -515,6 +515,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv) repr_priv = kzalloc(sizeof(*repr_priv), GFP_KERNEL); if (!repr_priv) { err = -ENOMEM; + nfp_repr_free(repr); goto err_reprs_clean; } @@ -525,11 +526,13 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv) port = nfp_port_alloc(app, NFP_PORT_PHYS_PORT, repr); if (IS_ERR(port)) { err = PTR_ERR(port); + kfree(repr_priv); nfp_repr_free(repr); goto err_reprs_clean; } err = nfp_port_init_phy_port(app->pf, app, port, i); if (err) { + kfree(repr_priv); nfp_port_free(port); nfp_repr_free(repr); goto err_reprs_clean; @@ -542,6 +545,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv) err = nfp_repr_init(app, repr, cmsg_port_id, port, priv->nn->dp.netdev); if (err) { + kfree(repr_priv); nfp_port_free(port); nfp_repr_free(repr); goto err_reprs_clean;
In nfp_flower_spawn_phy_reprs, in the for loop over eth_tbl if any of intermediate allocations or initializations fail memory is leaked. requiered releases are added. Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com> --- drivers/net/ethernet/netronome/nfp/flower/main.c | 4 ++++ 1 file changed, 4 insertions(+)