Re: [PATCH net 1/3] net/sched: act_ipt: add sanity checks on table name and hook locations

[Simon Horman <simon.horman@corigine.com>]

On Wed, Jun 07, 2023 at 04:59:52PM +0200, Florian Westphal wrote:
> Looks like "tc" hard-codes "mangle" as the only supported table
> name, but on kernel side there are no checks.
> 
> This is wrong.  Not all xtables targets are safe to call from tc.
> E.g. "nat" targets assume skb has a conntrack object assigned to it.
> Normally those get called from netfilter nat core which consults the
> nat table to obtain the address mapping.
> 
> "tc" userspace either sets PRE or POSTROUTING as hook number, but there
> is no validation of this on kernel side, so update netlink policy to
> reject bogus numbers.  Some targets may assume skb_dst is set for
> input/forward hooks, so prevent those from being used.
> 
> act_ipt uses the hook number in two places:
> 1. the state hook number, this is fine as-is
> 2. to set par.hook_mask
> 
> The latter is a bit mask, so update the assignment to make
> xt_check_target() to the right thing.
> 
> Followup patch adds required checks for the skb/packet headers before
> calling the targets evaluation function.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Hi Florian,

I think that patches for 'net' usually come with a fixes tag.
Likewise for the other patches in this series.

That aside, this looks good to me.

Reviewed-by: Simon Horman <simon.horman@corigine.com>