| Message ID | 1612852669-4165-1-git-send-email-wenxu@ucloud.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1bcc51ac0731aab1b109b2cd5c3d495f1884e5ca |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v5] net/sched: cls_flower: Reject invalid ct_state flags rules | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 6 maintainers not CCed: xiyou.wangcong@gmail.com davem@davemloft.net paulb@mellanox.com jiri@resnulli.us marcelo.leitner@gmail.com yossiku@mellanox.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 7176 this patch: 7176 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 77 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 7581 this patch: 7581 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Feb 09, 2021 at 02:37:49PM +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_MAX \ > + ((__TCA_FLOWER_KEY_CT_FLAGS_MAX - 1) << 1) > +#define TCA_FLOWER_KEY_CT_FLAGS_MASK \ > + (TCA_FLOWER_KEY_CT_FLAGS_MAX - 1) > + > struct fl_flow_key { > struct flow_dissector_key_meta meta; > struct flow_dissector_key_control control; > @@ -686,8 +691,10 @@ static void *fl_get(struct tcf_proto *tp, u32 handle) > [TCA_FLOWER_KEY_ENC_IP_TTL_MASK] = { .type = NLA_U8 }, > [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 }, > - [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NLA_U16 }, > + [TCA_FLOWER_KEY_CT_STATE] = > + NLA_POLICY_MASK(NLA_U16, TCA_FLOWER_KEY_CT_FLAGS_MASK), > + [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 }, > @@ -1390,12 +1397,33 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key, > return 0; > } > > +static int fl_validate_ct_state(u16 state, struct nlattr *tb, > + struct netlink_ext_ack *extack) > +{ > + if (state && !(state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED)) { > + NL_SET_ERR_MSG_ATTR(extack, tb, > + "no trk, so no other flag can be set"); I just tested iproute2 and it can't report based on the attr here. Nonetheless, that would be iproute2 job and not the errmsg, I think. Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Thanks! > + return -EINVAL; > + } > + > + if (state & TCA_FLOWER_KEY_CT_FLAGS_NEW && > + state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) { > + NL_SET_ERR_MSG_ATTR(extack, tb, > + "new and est are mutually exclusive"); > + return -EINVAL; > + } > + > + return 0; > +} > + > static int fl_set_key_ct(struct nlattr **tb, > struct flow_dissector_key_ct *key, > struct flow_dissector_key_ct *mask, > struct netlink_ext_ack *extack) > { > if (tb[TCA_FLOWER_KEY_CT_STATE]) { > + int err; > + > if (!IS_ENABLED(CONFIG_NF_CONNTRACK)) { > NL_SET_ERR_MSG(extack, "Conntrack isn't enabled"); > return -EOPNOTSUPP; > @@ -1403,6 +1431,13 @@ static int fl_set_key_ct(struct nlattr **tb, > fl_set_key_val(tb, &key->ct_state, TCA_FLOWER_KEY_CT_STATE, > &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK, > sizeof(key->ct_state)); > + > + err = fl_validate_ct_state(mask->ct_state, > + tb[TCA_FLOWER_KEY_CT_STATE_MASK], > + extack); > + if (err) > + return err; > + > } > if (tb[TCA_FLOWER_KEY_CT_ZONE]) { > if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) { > -- > 1.8.3.1 >
On Tue, 9 Feb 2021 14:37:49 +0800 wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > Reject the unsupported and invalid ct_state flags of cls flower rules. > > Fixes: e0ace68af2ac ("net/sched: cls_flower: Add matching on conntrack info") > Signed-off-by: wenxu <wenxu@ucloud.cn> As long as Cong is okay with the messages: Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Tue, 9 Feb 2021 14:37:49 +0800 you wrote: > From: wenxu <wenxu@ucloud.cn> > > Reject the unsupported and invalid ct_state flags of cls flower rules. > > Fixes: e0ace68af2ac ("net/sched: cls_flower: Add matching on conntrack info") > Signed-off-by: wenxu <wenxu@ucloud.cn> > > [...] Here is the summary with links: - [net,v5] net/sched: cls_flower: Reject invalid ct_state flags rules https://git.kernel.org/netdev/net/c/1bcc51ac0731 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h index ee95f42..88f4bf0 100644 --- a/include/uapi/linux/pkt_cls.h +++ b/include/uapi/linux/pkt_cls.h @@ -591,6 +591,8 @@ enum { TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED = 1 << 1, /* Part of an existing connection. */ TCA_FLOWER_KEY_CT_FLAGS_RELATED = 1 << 2, /* Related to an established connection. */ TCA_FLOWER_KEY_CT_FLAGS_TRACKED = 1 << 3, /* Conntrack has occurred. */ + + __TCA_FLOWER_KEY_CT_FLAGS_MAX, }; enum { diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 84f9325..46c1b3e 100644 --- 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_MAX \ + ((__TCA_FLOWER_KEY_CT_FLAGS_MAX - 1) << 1) +#define TCA_FLOWER_KEY_CT_FLAGS_MASK \ + (TCA_FLOWER_KEY_CT_FLAGS_MAX - 1) + struct fl_flow_key { struct flow_dissector_key_meta meta; struct flow_dissector_key_control control; @@ -686,8 +691,10 @@ static void *fl_get(struct tcf_proto *tp, u32 handle) [TCA_FLOWER_KEY_ENC_IP_TTL_MASK] = { .type = NLA_U8 }, [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 }, - [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NLA_U16 }, + [TCA_FLOWER_KEY_CT_STATE] = + NLA_POLICY_MASK(NLA_U16, TCA_FLOWER_KEY_CT_FLAGS_MASK), + [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 }, @@ -1390,12 +1397,33 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key, return 0; } +static int fl_validate_ct_state(u16 state, struct nlattr *tb, + struct netlink_ext_ack *extack) +{ + if (state && !(state & TCA_FLOWER_KEY_CT_FLAGS_TRACKED)) { + NL_SET_ERR_MSG_ATTR(extack, tb, + "no trk, so no other flag can be set"); + return -EINVAL; + } + + if (state & TCA_FLOWER_KEY_CT_FLAGS_NEW && + state & TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED) { + NL_SET_ERR_MSG_ATTR(extack, tb, + "new and est are mutually exclusive"); + return -EINVAL; + } + + return 0; +} + static int fl_set_key_ct(struct nlattr **tb, struct flow_dissector_key_ct *key, struct flow_dissector_key_ct *mask, struct netlink_ext_ack *extack) { if (tb[TCA_FLOWER_KEY_CT_STATE]) { + int err; + if (!IS_ENABLED(CONFIG_NF_CONNTRACK)) { NL_SET_ERR_MSG(extack, "Conntrack isn't enabled"); return -EOPNOTSUPP; @@ -1403,6 +1431,13 @@ static int fl_set_key_ct(struct nlattr **tb, fl_set_key_val(tb, &key->ct_state, TCA_FLOWER_KEY_CT_STATE, &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK, sizeof(key->ct_state)); + + err = fl_validate_ct_state(mask->ct_state, + tb[TCA_FLOWER_KEY_CT_STATE_MASK], + extack); + if (err) + return err; + } if (tb[TCA_FLOWER_KEY_CT_ZONE]) { if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) {