Re: [PATCH] net: flow_offload: fix memory leak in nfp_abm_u32_knode_replace

[Jakub Kicinski]

On Wed, 25 Sep 2019 13:34:46 -0500, Navid Emamdoost wrote:
> In nfp_abm_u32_knode_replace if the allocation for match fails it should
> go to the error handling instead of returning.
> 
> Signed-off-by: Navid Emamdoost <navid.emamdoost@xxxxxxxxx>
> ---
>  drivers/net/ethernet/netronome/nfp/abm/cls.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/abm/cls.c b/drivers/net/ethernet/netronome/nfp/abm/cls.c
> index 23ebddfb9532..32eaab99d96c 100644
> --- a/drivers/net/ethernet/netronome/nfp/abm/cls.c
> +++ b/drivers/net/ethernet/netronome/nfp/abm/cls.c
> @@ -174,7 +174,7 @@ nfp_abm_u32_knode_replace(struct nfp_abm_link *alink,
>  	struct nfp_abm_u32_match *match = NULL, *iter;
>  	unsigned int tos_off;
>  	u8 mask, val;
> -	int err;
> +	int err, ret = -EOPNOTSUPP;

You can use the err variable for the return. Please don't break the
reverse christmas tree ordering. Please initialize the err variable 
in the branch where failure occurred, not at the start of the function.

>  	if (!nfp_abm_u32_check_knode(alink->abm, knode, proto, extack))
>  		goto err_delete;
> @@ -204,8 +204,11 @@ nfp_abm_u32_knode_replace(struct nfp_abm_link *alink,
>  
>  	if (!match) {
>  		match = kzalloc(sizeof(*match), GFP_KERNEL);
> -		if (!match)
> -			return -ENOMEM;
> +		if (!match) {
> +			ret = -ENOMEM;
> +			goto err_delete;
> +		}
> +
>  		list_add(&match->list, &alink->dscp_map);
>  	}
>  	match->handle = knode->handle;
> @@ -221,7 +224,7 @@ nfp_abm_u32_knode_replace(struct nfp_abm_link *alink,
>  
>  err_delete:
>  	nfp_abm_u32_knode_delete(alink, knode);
> -	return -EOPNOTSUPP;
> +	return ret;
>  }
>  
>  static int nfp_abm_setup_tc_block_cb(enum tc_setup_type type,