Re: [PATCH net v4] net/sched: cls_flower: Reject invalid ct_state flags rules

[Marcelo Ricardo Leitner <mleitner@redhat.com>]

On Sun, Feb 07, 2021 at 01:13:23PM +0800, wenxu@ucloud.cn wrote:
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -30,6 +30,11 @@
>  
>  #include <uapi/linux/netfilter/nf_conntrack_common.h>
>  
> +#define TCA_FLOWER_KEY_CT_FLAGS_MASK (TCA_FLOWER_KEY_CT_FLAGS_NEW | \
> +				      TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED | \
> +				      TCA_FLOWER_KEY_CT_FLAGS_RELATED | \
> +				      TCA_FLOWER_KEY_CT_FLAGS_TRACKED)
> +

I know Jakub had said the calculations for _MASK were complicated, but
seeing this, they seem worth, otherwise we have to manually maintain
this duplicated list of entries here.

Maybe add just the __TCA_FLOWER_KEY_CT_FLAGS_MAX to the enum, and do
the calcs here? (to avoid having them in uapi)

>  struct fl_flow_key {
>  	struct flow_dissector_key_meta meta;
>  	struct flow_dissector_key_control control;
> @@ -687,7 +692,8 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
>  	[TCA_FLOWER_KEY_ENC_OPTS]	= { .type = NLA_NESTED },
>  	[TCA_FLOWER_KEY_ENC_OPTS_MASK]	= { .type = NLA_NESTED },
>  	[TCA_FLOWER_KEY_CT_STATE]	= { .type = NLA_U16 },

I wonder if this one should be protected by the flags mask as well.
It won't take action on unknown bits because of the mask below, but
still, it is accepting data that it doesn't know its meaning.

> -	[TCA_FLOWER_KEY_CT_STATE_MASK]	= { .type = NLA_U16 },
> +	[TCA_FLOWER_KEY_CT_STATE_MASK]	=
> +		NLA_POLICY_MASK(NLA_U16, TCA_FLOWER_KEY_CT_FLAGS_MASK),
>  	[TCA_FLOWER_KEY_CT_ZONE]	= { .type = NLA_U16 },
>  	[TCA_FLOWER_KEY_CT_ZONE_MASK]	= { .type = NLA_U16 },
>  	[TCA_FLOWER_KEY_CT_MARK]	= { .type = NLA_U32 },