tc flower rules support to classify ESP/AH
packets matching SPI field.
Signed-off-by: Ratheesh Kannoth <[email protected]>
---
include/uapi/linux/pkt_cls.h | 3 +++
net/sched/cls_flower.c | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 7865f5a9885b..75506f157340 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -598,6 +598,9 @@ enum {
TCA_FLOWER_KEY_CFM, /* nested */
+ TCA_FLOWER_KEY_SPI, /* be32 */
+ TCA_FLOWER_KEY_SPI_MASK, /* be32 */
+
__TCA_FLOWER_MAX,
};
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 8da9d039d964..eca260272845 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -72,6 +72,7 @@ struct fl_flow_key {
struct flow_dissector_key_num_of_vlans num_of_vlans;
struct flow_dissector_key_pppoe pppoe;
struct flow_dissector_key_l2tpv3 l2tpv3;
+ struct flow_dissector_key_ipsec ipsec;
struct flow_dissector_key_cfm cfm;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
@@ -726,6 +727,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
[TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
[TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
[TCA_FLOWER_KEY_L2TPV3_SID] = { .type = NLA_U32 },
+ [TCA_FLOWER_KEY_SPI] = { .type = NLA_U32 },
+ [TCA_FLOWER_KEY_SPI_MASK] = { .type = NLA_U32 },
[TCA_FLOWER_L2_MISS] = NLA_POLICY_MAX(NLA_U8, 1),
[TCA_FLOWER_KEY_CFM] = { .type = NLA_NESTED },
};
@@ -795,6 +798,24 @@ static void fl_set_key_val(struct nlattr **tb,
nla_memcpy(mask, tb[mask_type], len);
}
+static int fl_set_key_spi(struct nlattr **tb, struct fl_flow_key *key,
+ struct fl_flow_key *mask,
+ struct netlink_ext_ack *extack)
+{
+ if (key->basic.ip_proto != IPPROTO_ESP &&
+ key->basic.ip_proto != IPPROTO_AH) {
+ NL_SET_ERR_MSG(extack,
+ "Protocol must be either ESP or AH");
+ return -EINVAL;
+ }
+
+ fl_set_key_val(tb, &key->ipsec.spi,
+ TCA_FLOWER_KEY_SPI,
+ &mask->ipsec.spi, TCA_FLOWER_KEY_SPI_MASK,
+ sizeof(key->ipsec.spi));
+ return 0;
+}
+
static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
struct fl_flow_key *mask,
struct netlink_ext_ack *extack)
@@ -1894,6 +1915,12 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
return ret;
}
+ if (tb[TCA_FLOWER_KEY_SPI]) {
+ ret = fl_set_key_spi(tb, key, mask, extack);
+ if (ret)
+ return ret;
+ }
+
if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -2066,6 +2093,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_PPPOE, pppoe);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_L2TPV3, l2tpv3);
+ FL_KEY_SET_IF_MASKED(mask, keys, cnt,
+ FLOW_DISSECTOR_KEY_IPSEC, ipsec);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_CFM, cfm);
@@ -3364,6 +3393,12 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
sizeof(key->l2tpv3.session_id)))
goto nla_put_failure;
+ if (key->ipsec.spi &&
+ fl_dump_key_val(skb, &key->ipsec.spi, TCA_FLOWER_KEY_SPI,
+ &mask->ipsec.spi, TCA_FLOWER_KEY_SPI_MASK,
+ sizeof(key->ipsec.spi)))
+ goto nla_put_failure;
+
if ((key->basic.ip_proto == IPPROTO_TCP ||
key->basic.ip_proto == IPPROTO_UDP ||
key->basic.ip_proto == IPPROTO_SCTP) &&
--
2.25.1
+ Dan Carpenter
On Tue, Aug 01, 2023 at 07:10:59AM +0530, Ratheesh Kannoth wrote:
> tc flower rules support to classify ESP/AH
> packets matching SPI field.
>
> Signed-off-by: Ratheesh Kannoth <[email protected]>
> ---
> include/uapi/linux/pkt_cls.h | 3 +++
> net/sched/cls_flower.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 7865f5a9885b..75506f157340 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -598,6 +598,9 @@ enum {
>
> TCA_FLOWER_KEY_CFM, /* nested */
>
> + TCA_FLOWER_KEY_SPI, /* be32 */
> + TCA_FLOWER_KEY_SPI_MASK, /* be32 */
> +
> __TCA_FLOWER_MAX,
> };
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 8da9d039d964..eca260272845 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -72,6 +72,7 @@ struct fl_flow_key {
> struct flow_dissector_key_num_of_vlans num_of_vlans;
> struct flow_dissector_key_pppoe pppoe;
> struct flow_dissector_key_l2tpv3 l2tpv3;
> + struct flow_dissector_key_ipsec ipsec;
> struct flow_dissector_key_cfm cfm;
> } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>
> @@ -726,6 +727,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> [TCA_FLOWER_KEY_PPPOE_SID] = { .type = NLA_U16 },
> [TCA_FLOWER_KEY_PPP_PROTO] = { .type = NLA_U16 },
> [TCA_FLOWER_KEY_L2TPV3_SID] = { .type = NLA_U32 },
> + [TCA_FLOWER_KEY_SPI] = { .type = NLA_U32 },
> + [TCA_FLOWER_KEY_SPI_MASK] = { .type = NLA_U32 },
> [TCA_FLOWER_L2_MISS] = NLA_POLICY_MAX(NLA_U8, 1),
> [TCA_FLOWER_KEY_CFM] = { .type = NLA_NESTED },
> };
> @@ -795,6 +798,24 @@ static void fl_set_key_val(struct nlattr **tb,
> nla_memcpy(mask, tb[mask_type], len);
> }
>
> +static int fl_set_key_spi(struct nlattr **tb, struct fl_flow_key *key,
> + struct fl_flow_key *mask,
> + struct netlink_ext_ack *extack)
> +{
> + if (key->basic.ip_proto != IPPROTO_ESP &&
> + key->basic.ip_proto != IPPROTO_AH) {
> + NL_SET_ERR_MSG(extack,
> + "Protocol must be either ESP or AH");
> + return -EINVAL;
> + }
> +
> + fl_set_key_val(tb, &key->ipsec.spi,
> + TCA_FLOWER_KEY_SPI,
> + &mask->ipsec.spi, TCA_FLOWER_KEY_SPI_MASK,
> + sizeof(key->ipsec.spi));
> + return 0;
> +}
> +
> static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
> struct fl_flow_key *mask,
> struct netlink_ext_ack *extack)
> @@ -1894,6 +1915,12 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> return ret;
> }
>
> + if (tb[TCA_FLOWER_KEY_SPI]) {
> + ret = fl_set_key_spi(tb, key, mask, extack);
> + if (ret)
> + return ret;
> + }
> +
Hi Dan,
I'm seeing a warning from Smatch, which I think is a false positive,
but I feel that I should raise. Perhaps you could take a look at it?
net/sched/cls_flower.c:1918 fl_set_key() error: buffer overflow 'tb' 106 <= 108
> if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
> tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
> key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
> @@ -2066,6 +2093,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
> FLOW_DISSECTOR_KEY_PPPOE, pppoe);
> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> FLOW_DISSECTOR_KEY_L2TPV3, l2tpv3);
> + FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> + FLOW_DISSECTOR_KEY_IPSEC, ipsec);
> FL_KEY_SET_IF_MASKED(mask, keys, cnt,
> FLOW_DISSECTOR_KEY_CFM, cfm);
>
> @@ -3364,6 +3393,12 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
> sizeof(key->l2tpv3.session_id)))
> goto nla_put_failure;
>
> + if (key->ipsec.spi &&
> + fl_dump_key_val(skb, &key->ipsec.spi, TCA_FLOWER_KEY_SPI,
> + &mask->ipsec.spi, TCA_FLOWER_KEY_SPI_MASK,
> + sizeof(key->ipsec.spi)))
> + goto nla_put_failure;
> +
> if ((key->basic.ip_proto == IPPROTO_TCP ||
> key->basic.ip_proto == IPPROTO_UDP ||
> key->basic.ip_proto == IPPROTO_SCTP) &&
> --
> 2.25.1
>
>
On Wed, Aug 02, 2023 at 09:07:35PM +0200, Simon Horman wrote:
> + Dan Carpenter
>
> On Tue, Aug 01, 2023 at 07:10:59AM +0530, Ratheesh Kannoth wrote:
> > @@ -1894,6 +1915,12 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
> > return ret;
> > }
> >
> > + if (tb[TCA_FLOWER_KEY_SPI]) {
> > + ret = fl_set_key_spi(tb, key, mask, extack);
> > + if (ret)
> > + return ret;
> > + }
> > +
>
> Hi Dan,
>
> I'm seeing a warning from Smatch, which I think is a false positive,
> but I feel that I should raise. Perhaps you could take a look at it?
>
> net/sched/cls_flower.c:1918 fl_set_key() error: buffer overflow 'tb' 106 <= 108
>
You're using the cross function database, right? What happens is that
when someone adds a new type of net link attribute, it takes a rebuild
for the database to sync up.
I can't think of a good way to fix this. This information is passed as
a BUF_SIZE. Each database rebuild passes the BUF_SIZE one call further
down the call tree.
$ smdb fl_set_key | grep BUF_SIZE
net/sched/cls_flower.c | fl_change | fl_set_key | BUF_SIZE | 1 | tb | 864
net/sched/cls_flower.c | fl_tmplt_create | fl_set_key | BUF_SIZE | 1 | tb | 864
This is a flaw in how Smatch works, and theoretically it affects
everything, but in practical terms it affect netlink attribute tables
the most. Other places are not modified as often or they pass the size
as a parameter. I could modify check_index_overflow.c to silence
warnings where it's a netlink attribute table and the offset is less
than __TCA_FLOWER_MAX.
regards,
dan carpenter
Done. :) That false positive has been bothering me for a while so it's
nice to have it fixed. I'll test this out for a bit before pushing.
regards,
dan carpenter
diff --git a/check_index_overflow.c b/check_index_overflow.c
index 19ea4354029b..644310ae837c 100644
--- a/check_index_overflow.c
+++ b/check_index_overflow.c
@@ -160,6 +160,43 @@ free:
return ret;
}
+static unsigned long __TCA_FLOWER_MAX(void)
+{
+ struct symbol *sym;
+ struct ident *id;
+ sval_t sval;
+
+ id = built_in_ident("__TCA_FLOWER_MAX");
+ sym = lookup_symbol(id, NS_SYMBOL);
+ if (!sym)
+ return 0;
+ if (!get_value(sym->initializer, &sval))
+ return 0;
+ return sval.value;
+}
+
+static bool is_out_of_sync_nla_tb(struct expression *array_expr, struct expression *offset)
+{
+ sval_t sval;
+ char *type;
+
+ if (option_project != PROJ_KERNEL)
+ return false;
+
+ if (!get_value(offset, &sval))
+ return false;
+ type = type_to_str(get_type(array_expr));
+ if (!type)
+ return false;
+ if (strcmp(type, "struct nlattr**") != 0)
+ return false;
+
+ if (sval.uvalue >= __TCA_FLOWER_MAX())
+ return false;
+
+ return true;
+}
+
static int is_subtract(struct expression *expr)
{
struct expression *tmp;
@@ -286,6 +323,9 @@ static int should_warn(struct expression *expr)
if (common_false_positives(array_expr, max))
return 0;
+ if (is_out_of_sync_nla_tb(array_expr, offset))
+ return 0;
+
if (impossibly_high_comparison(offset))
return 0;