From: Andrey Mokrushin <[email protected]>
rte_acl_build() doesn't check that for a RANGE field the low bound
(value) doesn't exceed the high bound (mask_range), and
rte_acl_add_rules() can't check it, as it doesn't know the field
types. Such a rule is accepted, and then:
- for a 1-byte field the build crashes with a NULL pointer
dereference in acl_gen_node();
- for a 2-byte field (e.g. port range 80..22) the build succeeds,
but the rule matches every value of the field instead of none.
The check was done only for the legacy ipv4vlan rules
(acl_ipv4vlan_check_rule(), now in app/test). Validate RANGE fields
in acl_build_rules(), where the configuration is known, and fail
the build with -EINVAL. Add a test case with the generic API.
Fixes: dc276b5780c2 ("acl: new library")
Cc: [email protected]
Signed-off-by: Andrey Mokrushin <[email protected]>
---
app/test/test_acl.c | 26 ++++++++++++++++++++++++
lib/acl/acl_bld.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/app/test/test_acl.c b/app/test/test_acl.c
index 5d207c8..47786bc 100644
--- a/app/test/test_acl.c
+++ b/app/test/test_acl.c
@@ -1274,6 +1274,7 @@ test_invalid_rules(void)
int ret;
struct rte_acl_ipv4vlan_rule rule;
+ struct acl_ipv4vlan_rule rv;
acx = rte_acl_create(&acl_param);
if (acx == NULL) {
@@ -1338,6 +1339,31 @@ test_invalid_rules(void)
goto err;
}
+ /* test inverted high/low destination ports added through the
+ * generic API: rte_acl_add_rules() doesn't know the field types,
+ * so the rule has to be rejected by rte_acl_build().
+ */
+ rule = acl_rule;
+ rule.data.userdata = 1;
+ rule.dst_port_low = 0xfff0;
+ rule.dst_port_high = 0x0010;
+
+ acl_ipv4vlan_convert_rule(&rule, &rv);
+ ret = rte_acl_add_rules(acx, (struct rte_acl_rule *)&rv, 1);
+ if (ret != 0) {
+ printf("Line %i: Adding rules to ACL context failed!\n",
+ __LINE__);
+ goto err;
+ }
+
+ ret = rte_acl_ipv4vlan_build(acx, ipv4_7tuple_layout,
+ RTE_ACL_MAX_CATEGORIES);
+ if (ret != -EINVAL) {
+ printf("Line %i: Building ACL context "
+ "should have failed!\n", __LINE__);
+ goto err;
+ }
+
rte_acl_free(acx);
return 0;
diff --git a/lib/acl/acl_bld.c b/lib/acl/acl_bld.c
index 99d1dbc..aa25c78 100644
--- a/lib/acl/acl_bld.c
+++ b/lib/acl/acl_bld.c
@@ -1443,6 +1443,52 @@ acl_build_log(const struct acl_build_context *ctx)
}
}
+/*
+ * Check that for all RANGE fields lower bound doesn't exceed upper bound.
+ */
+static int
+acl_check_rule_ranges(const struct rte_acl_config *cfg,
+ const struct rte_acl_rule *rule)
+{
+ uint32_t n;
+ uint64_t lo, hi;
+ const struct rte_acl_field *fld;
+
+ for (n = 0; n != cfg->num_fields; n++) {
+ if (cfg->defs[n].type != RTE_ACL_FIELD_TYPE_RANGE)
+ continue;
+
+ fld = rule->field + cfg->defs[n].field_index;
+ switch (cfg->defs[n].size) {
+ case sizeof(uint8_t):
+ lo = fld->value.u8;
+ hi = fld->mask_range.u8;
+ break;
+ case sizeof(uint16_t):
+ lo = fld->value.u16;
+ hi = fld->mask_range.u16;
+ break;
+ case sizeof(uint32_t):
+ lo = fld->value.u32;
+ hi = fld->mask_range.u32;
+ break;
+ default:
+ lo = fld->value.u64;
+ hi = fld->mask_range.u64;
+ break;
+ }
+
+ if (lo > hi) {
+ ACL_LOG(ERR,
+ "Error in rule[%u]: invalid range for %u-th
field",
+ rule->data.userdata, n);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int
acl_build_rules(struct acl_build_context *bcx)
{
@@ -1467,6 +1513,8 @@ acl_build_rules(struct acl_build_context *bcx)
rule = (const struct rte_acl_rule *)
((uintptr_t)bcx->acx->rules + bcx->acx->rule_sz * i);
if ((rule->data.category_mask & bcx->category_mask) != 0) {
+ if (acl_check_rule_ranges(&bcx->cfg, rule) != 0)
+ return -EINVAL;
br[num].next = head;
br[num].config = &bcx->cfg;
br[num].f = rule;
--
2.56.0
