Re: [PATCH net] netfilter: nf_flow_table_offload: fix heap overflow in flow_action_entry_next()

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> Hyunwoo Kim <imv4bel@gmail.com> wrote:
> > flow_action_entry_next() increments num_entries and returns a pointer
> > into the flow_action_entry array without any bounds checking.  The array
> > is allocated with a fixed size of NF_FLOW_RULE_ACTION_MAX (16) entries,
> > but certain combinations of IPv6 + SNAT + DNAT + double VLAN (QinQ)
> > require 17 or more entries, causing a slab-out-of-bounds write in the
> > kmalloc-4k slab.
> > 
> > The maximum possible entry count is:
> >   tunnel(2) + eth(4) + VLAN(4) + IPv6_NAT(10) + redirect(1) = 21
> > 
> > Increase NF_FLOW_RULE_ACTION_MAX to 24 (with headroom) to cover the
> >  
> > -#define NF_FLOW_RULE_ACTION_MAX	16
> > +#define NF_FLOW_RULE_ACTION_MAX	24
> 
> This fix looks rather fragile.
> 
> What guarantees that this stays right-sized?
> 
> Can you add a BUILD_BUG_ON or if needed, run-time check?

Ping.  I'm not even sure if there is a bug to begin with, see Pablos
response.  How did you conclude there is a missing bounds check and that
this increase is the best fix?

Normally there should be a check that prevents such a configuration.
If thats missing, please add one instead of increasing this define.