netdev - Re: [PATCH net v4] net/sched: cls_flower: Reject invalid ct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210208185705.GE2953@horizon.localdomain>
Date:   Mon, 8 Feb 2021 15:57:05 -0300
From:   Marcelo Ricardo Leitner <mleitner@...hat.com>
To:     wenxu@...oud.cn
Cc:     jhs@...atatu.com, kuba@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net v4] net/sched: cls_flower: Reject invalid ct_state
 flags rules

On Sun, Feb 07, 2021 at 01:13:23PM +0800, wenxu@...oud.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 },