Re: [PATCH nf-next 1/3] netfilter: nf_tables: add userdata attributes to nft_chain

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

On Mon, Sep 21, 2020 at 03:28:21PM +0200, Jose M. Guisado Gomez 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>
> ---
>  include/net/netfilter/nf_tables.h        |  2 ++
>  include/uapi/linux/netfilter/nf_tables.h |  2 ++
>  net/netfilter/nf_tables_api.c            | 19 +++++++++++++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 8ceca0e419b3..4686fafbfd8a 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -952,6 +952,8 @@ struct nft_chain {
>  					bound:1,
>  					genmask:2;
>  	char				*name;
> +	u16				udlen;
> +	u8				*udata;
>  
>  	/* Only used during control plane commit phase: */
>  	struct nft_rule			**rules_next;
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 3c2469b43742..352ee51707a1 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -208,6 +208,7 @@ enum nft_chain_flags {
>   * @NFTA_CHAIN_COUNTERS: counter specification of the chain (NLA_NESTED: nft_counter_attributes)
>   * @NFTA_CHAIN_FLAGS: chain flags
>   * @NFTA_CHAIN_ID: uniquely identifies a chain in a transaction (NLA_U32)
> + * @NFTA_CHAIN_USERDATA: user data (NLA_BINARY)
>   */
>  enum nft_chain_attributes {
>  	NFTA_CHAIN_UNSPEC,
> @@ -222,6 +223,7 @@ enum nft_chain_attributes {
>  	NFTA_CHAIN_PAD,
>  	NFTA_CHAIN_FLAGS,
>  	NFTA_CHAIN_ID,
> +	NFTA_CHAIN_USERDATA,
>  	__NFTA_CHAIN_MAX
>  };
>  #define NFTA_CHAIN_MAX		(__NFTA_CHAIN_MAX - 1)
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 84c0c1aaae99..c8065c6eae86 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
[...]
> @@ -2052,6 +2059,18 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
>  		goto err1;
>  	}
>  
> +	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 err1;
> +		}
> +
> +		nla_memcpy(chain->udata, nla[NFTA_CHAIN_USERDATA], udlen);
> +		chain->udlen = udlen;
> +	}
> +
>  	rules = nf_tables_chain_alloc_rules(chain, 0);
>  	if (!rules) {
>  		err = -ENOMEM;

Hm, kfree(chain->udata) from the error path is missing?

While working at this, probably you can rename all those ugly err1;
basic-like goto style in the same patch.

Thanks.