Re: [PATCH nf] netfilter: nft_set_pipapo: reject mismatched sum of field_len with key length

[Florian Westphal <fw@xxxxxxxxx>]

Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> The field length description provides the length of each separated key
> fields in the concatenation. The set key length provides the total size
> of the key aligned to 32-bits for the pipapo set backend. Reject with
> EINVAL if the field length description and set key length provided by
> userspace are inconsistent.
> 
> Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
>  net/netfilter/nft_set_pipapo.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index 7be342b495f5..3b1a53e68989 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -2235,6 +2235,7 @@ static int nft_pipapo_init(const struct nft_set *set,
>  	struct nft_pipapo_match *m;
>  	struct nft_pipapo_field *f;
>  	int err, i, field_count;
> +	unsigned int len = 0;
>  
>  	BUILD_BUG_ON(offsetof(struct nft_pipapo_elem, priv) != 0);
>  
> @@ -2246,6 +2247,12 @@ static int nft_pipapo_init(const struct nft_set *set,
>  	if (field_count > NFT_PIPAPO_MAX_FIELDS)
>  		return -EINVAL;
>  
> +	for (i = 0; i < field_count; i++)
> +		len += round_up(desc->field_len[i], sizeof(u32));
> +
> +	if (len != set->klen)
> +		return -EINVAL;
> +

I fail to grasp why nft_set_desc_concat() doesn't catch it:

        for (i = 0; i < desc->field_count; i++)
                num_regs += DIV_ROUND_UP(desc->field_len[i], sizeof(u32));

        key_num_regs = DIV_ROUND_UP(desc->klen, sizeof(u32));
        if (key_num_regs != num_regs);	----> here....
                return -EINVAL;