Re: [libnftnl PATCH 3/6] set_elem: Validate nftnl_set_elem_set() parameters

[Phil Sutter <phil@nwl.cc>]

Hi,

On Tue, Oct 15, 2019 at 06:09:13PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 15, 2019 at 06:02:55PM +0200, Phil Sutter wrote:
> > On Tue, Oct 15, 2019 at 05:52:44PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Oct 15, 2019 at 04:16:55PM +0200, Phil Sutter wrote:
> > [...]
> > > > diff --git a/src/set_elem.c b/src/set_elem.c
> > > > index 3794f12594079..4225a96ee5a0a 100644
> > > > --- a/src/set_elem.c
> > > > +++ b/src/set_elem.c
> > > > @@ -96,10 +96,20 @@ void nftnl_set_elem_unset(struct nftnl_set_elem *s, uint16_t attr)
> > > >  	s->flags &= ~(1 << attr);
> > > >  }
> > > >  
> > > > +static uint32_t nftnl_set_elem_validate[NFTNL_SET_ELEM_MAX + 1] = {
> > > > +	[NFTNL_SET_ELEM_FLAGS]		= sizeof(uint32_t),
> > > > +	[NFTNL_SET_ELEM_VERDICT]	= sizeof(int), /* FIXME: data.verdict is int?! */
> > > 
> > > This is uint32_t, update this before pushing out this.
> > 
> > Oh, sorry. I missed this note to myself.
> > 
> > So, should we change union nftnl_data_reg accordingly then?
> 
> I'm seeing this is being used from nftables.git as...
> 
>         nftnl_set_elem_set_u32(nlse, NFTNL_SET_ELEM_VERDICT, ...

Well, there's no nftnl_set_elem_set_int() so it naturally uses that. My
question was whether 'verdict' field in union nftnl_data_reg should be
changed to uint32_t type as well. Currently it is just int, which
doesn't make a difference unless one tries to run nftables on a 16bit
machine. :)

Cheers, Phil