Re: [PATCH v3 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain

[Florian Westphal <fw@strlen.de>]

Jose M. Guisado Gomez <guigom@riseup.net> wrote:
> Enables storing userdata for nft_chain. Field udata points to user data
> and udlen stores its length.
> 
> Adds new attribute flag NFTA_CHAIN_USERDATA.
> 
> Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
> ---
> +	if (nla[NFTA_CHAIN_USERDATA]) {
> +		udlen = nla_len(nla[NFTA_CHAIN_USERDATA]);
> +		chain->udata = kzalloc(udlen, GFP_KERNEL);
> +		if (chain->udata == NULL) {
> +			err = -ENOMEM;
> +			goto err_destroy_chain;
> +		}
> +
> +		nla_memcpy(chain->udata, nla[NFTA_CHAIN_USERDATA], udlen);
> +		chain->udlen = udlen;

nit: You could use nla_memdup() instead of alloc+memcpy.

> -err2:
> +err_unregister_hook:
>  	nf_tables_unregister_hook(net, table, chain);
> -err1:
> +err_free_udata:
> +	kfree(chain->udata);
> +err_destroy_chain:
>  	nf_tables_chain_destroy(ctx);

This frees ->udata on error.  But what if the chain is added
successfully and then deleted at a later time?

Wouldn't it make more sense to only patch nf_tables_chain_destroy()
to handle both error and chain delete case?