Re: [PATCH nf 2/3] netfilter: nf_tables: memleak in nft_expr_init() path

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Jun 02, 2022 at 09:43:56AM +0200, Pablo Neira Ayuso wrote:
> Since ed0a0c60f0e5 ("netfilter: nft_quota: move stateful fields out of
> expression data"), stateful expressions allocate stateful area via
> kmalloc().
> 
> Call ->ops->destroy() to release expression stateful information before
> releasing the expression.
> 
> ->ops->deactivate() is also called for safety reasons, no stateful
> expressions support for this function.
> 
> Fixes: 6cafaf4764a3 ("netfilter: nf_tables: fix memory leak if expr init fails")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_tables_api.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 5ee4e7b28b61..017b77067852 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2904,6 +2904,11 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
>  
>  	return expr;
>  err_expr_new:
> +	if (expr->ops->deactivate)
> +		expr->ops->deactivate(ctx, expr, NFT_TRANS_RELEASE);
> +	if (expr_info.ops->destroy)
> +		expr_info.ops->destroy(ctx, expr);

Scratch this patch. Not correct.

nf_tables_newexpr() calls ->init() which if it fails, then it did not
allocate anything.

> +
>  	kfree(expr);
>  err_expr_stateful:
>  	owner = expr_info.ops->type->owner;
> -- 
> 2.30.2
>