On Tue, Jan 28, 2025 at 01:30:12AM +0100, Florian Westphal wrote:
> 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;
This check is loose, I will post a v2 fixing up nft_set_desc_concat().
Thanks.
