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

[Florian Westphal <fw@strlen.de>]

Hyunwoo Kim <imv4bel@gmail.com> wrote:
> hmm. So, based on what you said, I assume the run-time check would look 
> something like this?
> 
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 9b677e116487..69ffefbdd5e8 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -218,6 +218,9 @@ flow_action_entry_next(struct nf_flow_rule *flow_rule)
>  {
>         int i = flow_rule->rule->action.num_entries++;
> 
> +       if (WARN_ON_ONCE(i >= NF_FLOW_RULE_ACTION_MAX))
> +               return NULL;
> +
>         return &flow_rule->rule->action.entries[i];
>  }
> 
> However, if we add a runtime check in this way, all callers of 
> flow_action_entry_next() would also need to handle a NULL return value, 
> since none of them currently perform a null check.
> 
> Because of the potential risk, this would require modifying quite a number 
> of call sites carefully. What do you think about this approach?

Can't we reject this at configuration time?

I mean, userspace has to ask for this action sequence, no?