Re: [PATCH] netfilter: nf_tables: set names cannot be larger than 15 bytes

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Mar 24, 2014 at 11:41:05PM +0100, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 33045a5..43ae487 100644
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -1946,7 +1946,8 @@ static const struct nft_set_ops *nft_select_set_ops(const struct nlattr * const
> > >  
> > >  static const struct nla_policy nft_set_policy[NFTA_SET_MAX + 1] = {
> > >  	[NFTA_SET_TABLE]		= { .type = NLA_STRING },
> > > -	[NFTA_SET_NAME]			= { .type = NLA_STRING },
> > > +	[NFTA_SET_NAME]			= { .type = NLA_STRING,
> > > +					    .len = IFNAMSIZ - 1 },
> > 
> > Not related to your patch, but it looks to me as if all of these
> > should be NLA_NUL_STRING, no?
> 
> This is what I originally thought. But all of the nla_* functions use
> the nla_len to know length of the string coming from userspace, so
> they don't rely on the trailing nul-termination.

Indeed.  I reviewed this again but only 'bug' I spotted:

nf_tables_chain_type_lookup() does:
request_module("nft-chain-%u-%*.s", ....

('%.*s' was probably intended).  Its harmless though as NFTA_CHAIN_TYPE is
NLA_NUL_STR so the erroneous fmt could also be changed to plain %s.