netdev - Re: [PATCH] net: flow_offload: fix memory leak in nfp_abm_u32_knode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190925182846.69a261e8@cakuba.netronome.com>
Date:   Wed, 25 Sep 2019 18:28:46 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Navid Emamdoost <navid.emamdoost@...il.com>
Cc:     emamd001@....edu, smccaman@....edu, kjlu@....edu,
        "David S. Miller" <davem@...emloft.net>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Colin Ian King <colin.king@...onical.com>,
        oss-drivers@...ronome.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: flow_offload: fix memory leak in
 nfp_abm_u32_knode_replace

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@...il.com>
> ---
>  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,