| 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
| ||
|
Message-ID: <5dde483c-be42-644c-30b5-add308b4f69d@solarflare.com>
Date: Mon, 9 Mar 2020 17:36:30 +0000
From: Edward Cree <ecree@...arflare.com>
To: Paul Blakey <paulb@...lanox.com>,
Saeed Mahameed <saeedm@...lanox.com>,
Oz Shlomo <ozsh@...lanox.com>,
Jakub Kicinski <jakub.kicinski@...ronome.com>,
Vlad Buslov <vladbu@...lanox.com>,
"David Miller" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Jiri Pirko <jiri@...lanox.com>, Roi Dayan <roid@...lanox.com>
Subject: Re: [PATCH net-next ct-offload v2 02/13] net/sched: act_ct:
Instantiate flow table entry actions
On 08/03/2020 14:10, Paul Blakey wrote:
> NF flow table API associate 5-tuple rule with an action list by calling
> the flow table type action() CB to fill the rule's actions.
>
> In action CB of act_ct, populate the ct offload entry actions with a new
> ct_metadata action. Initialize the ct_metadata with the ct mark, label and
> zone information. If ct nat was performed, then also append the relevant
> packet mangle actions (e.g. ipv4/ipv6/tcp/udp header rewrites).
>
> Drivers that offload the ft entries may match on the 5-tuple and perform
> the action list.
>
> Signed-off-by: Paul Blakey <paulb@...lanox.com>
> Reviewed-by: Jiri Pirko <jiri@...lanox.com>
> ---
> Changelog:
> v1->v2:
> Remove zone from metadata
> Add add mangle helper func (removes the unneccasry () and correct the mask there)
> Remove "abuse" of ? operator and use switch case
> Check protocol and ports in relevant function and return err
> On error restore action entries (on the topic, validaiting num of action isn't available)
> Add comment expalining nat
> Remove Inlinie from tcf_ct_flow_table_flow_action_get_next
> Refactor tcf_ct_flow_table_add_action_nat_ipv6 with helper
> On nats, allow both src and dst mangles
>
> include/net/flow_offload.h | 5 +
> include/net/netfilter/nf_flow_table.h | 23 ++++
> net/netfilter/nf_flow_table_offload.c | 23 ----
> net/sched/act_ct.c | 208 ++++++++++++++++++++++++++++++++++
> 4 files changed, 236 insertions(+), 23 deletions(-)
>
> <snip>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 23eba61..d57e7969 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -55,7 +55,215 @@ struct tcf_ct_flow_table {
> .automatic_shrinking = true,
> };
>
> +static struct flow_action_entry *
> +tcf_ct_flow_table_flow_action_get_next(struct flow_action *flow_action)
> +{
> + int i = flow_action->num_entries++;
> +
> + return &flow_action->entries[i];
> +}
> +
> +static void tcf_ct_add_mangle_action(struct flow_action *action,
> + enum flow_action_mangle_base htype,
> + u32 offset,
> + u32 mask,
> + u32 val)
> +{
> + struct flow_action_entry *entry;
> +
> + entry = tcf_ct_flow_table_flow_action_get_next(action);
> + entry->id = FLOW_ACTION_MANGLE;
> + entry->mangle.htype = htype;
> + entry->mangle.mask = ~mask;
> + entry->mangle.offset = offset;
> + entry->mangle.val = val;
> +}
> +
> +static void
> +tcf_ct_flow_table_add_action_nat_ipv4(const struct nf_conntrack_tuple *tuple,
> + struct nf_conntrack_tuple target,
> + struct flow_action *action)
> +{
> + if (memcmp(&target.src.u3, &tuple->src.u3, sizeof(target.src.u3)))
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_IP4,
> + offsetof(struct iphdr, saddr),
> + 0xFFFFFF,
Why is this mask only 24 bits?
> + be32_to_cpu(target.src.u3.ip));
> + if (memcmp(&target.dst.u3, &tuple->dst.u3, sizeof(target.dst.u3)))
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_IP4,
> + offsetof(struct iphdr, daddr),
> + 0xFFFFFF,
> + be32_to_cpu(target.dst.u3.ip));
> +}
> +
> +static void
> +tcf_ct_add_ipv6_addr_mangle_action(struct flow_action *action,
> + union nf_inet_addr *addr,
> + u32 offset)
> +{
> + int i;
> +
> + for (i = 0; i < sizeof(struct in6_addr) / sizeof(u32); i++)
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_IP6,
> + i * sizeof(u32) + offset,
> + 0xFFFFFF, be32_to_cpu(addr->ip6[i]));
Again, looks like this is meant to be 0xffffffff.
> +}
> +
> +static void
> +tcf_ct_flow_table_add_action_nat_ipv6(const struct nf_conntrack_tuple *tuple,
> + struct nf_conntrack_tuple target,
> + struct flow_action *action)
> +{
> + if (memcmp(&target.src.u3, &tuple->src.u3, sizeof(target.src.u3)))
> + tcf_ct_add_ipv6_addr_mangle_action(action, &target.src.u3,
> + offsetof(struct ipv6hdr,
> + saddr));
> + if (memcmp(&target.dst.u3, &tuple->dst.u3, sizeof(target.dst.u3)))
> + tcf_ct_add_ipv6_addr_mangle_action(action, &target.dst.u3,
> + offsetof(struct ipv6hdr,
> + daddr));
> +}
> +
> +static void
> +tcf_ct_flow_table_add_action_nat_tcp(const struct nf_conntrack_tuple *tuple,
> + struct nf_conntrack_tuple target,
> + struct flow_action *action)
> +{
> + __be16 target_src = target.src.u.tcp.port;
> + __be16 target_dst = target.dst.u.tcp.port;
> +
> + if (target_src != tuple->src.u.tcp.port)
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_TCP,
> + offsetof(struct tcphdr, source),
> + 0xFFFF, htons(target_src));
htons() on a __be16 is wrong — did you run this through sparse?
(htons takes a u16 and returns a __be16: "host to network short".)
Either ntohs() or, canonically, be16_to_cpu().
> + if (target_dst != tuple->dst.u.tcp.port)
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_TCP,
> + offsetof(struct tcphdr, dest),
> + 0xFFFF, htons(target_dst));
> +}
> +
> +static void
> +tcf_ct_flow_table_add_action_nat_udp(const struct nf_conntrack_tuple *tuple,
> + struct nf_conntrack_tuple target,
> + struct flow_action *action)
> +{
> + __be16 target_src = target.src.u.udp.port;
> + __be16 target_dst = target.dst.u.udp.port;
> +
> + if (target_src != tuple->src.u.udp.port)
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_TCP,
> + offsetof(struct udphdr, source),
> + 0xFFFF, htons(target_src));
> + if (target_dst != tuple->dst.u.udp.port)
> + tcf_ct_add_mangle_action(action, FLOW_ACT_MANGLE_HDR_TYPE_TCP,
> + offsetof(struct udphdr, dest),
> + 0xFFFF, htons(target_dst));
> +}
> +
> +static void tcf_ct_flow_table_add_action_meta(struct nf_conn *ct,
> + enum ip_conntrack_dir dir,
> + struct flow_action *action)
> +{
> + struct nf_conn_labels *ct_labels;
> + struct flow_action_entry *entry;
> + u32 *act_ct_labels;
> +
> + entry = tcf_ct_flow_table_flow_action_get_next(action);
> + entry->id = FLOW_ACTION_CT_METADATA;
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
> + entry->ct_metadata.mark = ct->mark;
> +#endif
> +
> + act_ct_labels = entry->ct_metadata.labels;
> + ct_labels = nf_ct_labels_find(ct);
> + if (ct_labels)
> + memcpy(act_ct_labels, ct_labels->bits, NF_CT_LABELS_MAX_SIZE);
> + else
> + memset(act_ct_labels, 0, NF_CT_LABELS_MAX_SIZE);
> +}
> +
> +static int tcf_ct_flow_table_add_action_nat(struct net *net,
> + struct nf_conn *ct,
> + enum ip_conntrack_dir dir,
> + struct flow_action *action)
> +{
> + const struct nf_conntrack_tuple *tuple = &ct->tuplehash[dir].tuple;
> + struct nf_conntrack_tuple target;
> +
> + nf_ct_invert_tuple(&target, &ct->tuplehash[!dir].tuple);
> +
> + /* The following helper functions check if the inverted reverse tuple
> + * is different then the current dir tuple - meaning nat for ports
> + * and/or ip is needed, and add the relevant mangle actions.
> + */
Probably better to put this comment above the helpers, ratherthan the
calls to them, so that people reading the source linearly see the comment
before the memcmp()s.
-ed
> +
> + switch (tuple->src.l3num) {
> + case NFPROTO_IPV4:
> + tcf_ct_flow_table_add_action_nat_ipv4(tuple, target,
> + action);
> + break;
> + case NFPROTO_IPV6:
> + tcf_ct_flow_table_add_action_nat_ipv6(tuple, target,
> + action);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + switch (nf_ct_protonum(ct)) {
> + case IPPROTO_TCP:
> + tcf_ct_flow_table_add_action_nat_tcp(tuple, target, action);
> + break;
> + case IPPROTO_UDP:
> + tcf_ct_flow_table_add_action_nat_udp(tuple, target, action);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int tcf_ct_flow_table_fill_actions(struct net *net,
> + const struct flow_offload *flow,
> + enum flow_offload_tuple_dir tdir,
> + struct nf_flow_rule *flow_rule)
> +{
> + struct flow_action *action = &flow_rule->rule->action;
> + int num_entries = action->num_entries;
> + struct nf_conn *ct = flow->ct;
> + enum ip_conntrack_dir dir;
> + int i, err;
> +
> + switch (tdir) {
> + case FLOW_OFFLOAD_DIR_ORIGINAL:
> + dir = IP_CT_DIR_ORIGINAL;
> + break;
> + case FLOW_OFFLOAD_DIR_REPLY:
> + dir = IP_CT_DIR_REPLY;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + err = tcf_ct_flow_table_add_action_nat(net, ct, dir, action);
> + if (err)
> + goto err_nat;
> +
> + tcf_ct_flow_table_add_action_meta(ct, dir, action);
> + return 0;
> +
> +err_nat:
> + /* Clear filled actions */
> + for (i = num_entries; i < action->num_entries; i++)
> + memset(&action->entries[i], 0, sizeof(action->entries[i]));
> + action->num_entries = num_entries;
> +
> + return err;
> +}
> +
> static struct nf_flowtable_type flowtable_ct = {
> + .action = tcf_ct_flow_table_fill_actions,
> .owner = THIS_MODULE,
> };
>
Powered by blists - more mailing lists