[PATCH nft v2 0/8] really handle stacked l2 headers

[PATCH nft v2 0/8] really handle stacked l2 headers

[Florian Westphal]

v2:
- fix a UAF during rule listing.  When OP_AND gets culled,
  'expr' is free'd as well ahead of time because they alias one
  another in the set key case (there is no compare/relational op).
- add and handle plain 'vlan id' in a set key.
  in v1, this would be shown with the '& 0xfff' included, because
  v1 only removed OP_AND in concatenations.

Eric Garver reported a number of issues when matching vlan headers:

In:  update @macset { ether saddr . vlan id timeout 5s }
Out: update @macset { @ll,48,48 . @ll,112,16 & 0xfff timeout 5s }

This is because of amnesia in nft during expression decoding:
When we encounter 'vlan id', the L2 protocl (ethernet) is replaced by
vlan, so we attempt to match @ll,48,48 vs. the vlan header and come up
empty.

The vlan decode fails because we can't handle '& 0xfff' in this
instance, so we can locate the right offset but the payload expression
length doesn't match the template length (16 vs 12 bits).


The main patch is patch 3, which adds a stack of l2 protocols to track
instead of only keeping the cumulative size.

The latter is ok for serialization (we have the expression tree, so its
enough to add the size of the 'previous' l2 headers to payload
expressions that match the new 'top' l2 header.

But for deserialization, we need to be able to search all protocols base
headers seen.

The remaining patches improve handling of 'integer base type'
expressions and add test cases.

Florian Westphal (8):
  netlink_delinearize: allow postprocessing on concatenated elements
  netlink_delinearize: postprocess binary ands in concatenations
  proto: track full stack of seen l2 protocols, not just cumulative
    offset
  debug: dump the l2 protocol stack
  tests: add a test case for ether and vlan listing
  netlink_delinearize: also postprocess OP_AND in set element context
  evaluate: search stacked header list for matching payload dep
  src: allow anon set concatenation with ether and vlan

 include/netlink.h                             |  8 ++
 include/proto.h                               |  3 +-
 src/evaluate.c                                | 36 +++++--
 src/expression.c                              | 17 +++-
 src/netlink.c                                 | 10 +-
 src/netlink_delinearize.c                     | 59 ++++++++---
 src/payload.c                                 | 67 ++++++++++---
 src/proto.c                                   |  8 +-
 tests/py/bridge/vlan.t                        |  5 +
 tests/py/bridge/vlan.t.json                   | 97 +++++++++++++++++++
 tests/py/bridge/vlan.t.payload                | 28 ++++++
 tests/py/bridge/vlan.t.payload.netdev         | 34 +++++++
 .../testcases/sets/0070stacked_l2_headers     |  6 ++
 .../sets/dumps/0070stacked_l2_headers.nft     | 28 ++++++
 14 files changed, 368 insertions(+), 38 deletions(-)
 create mode 100755 tests/shell/testcases/sets/0070stacked_l2_headers
 create mode 100644 tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft

-- 
2.35.1

[PATCH nft v2 1/8] netlink_delinearize: allow postprocessing on concatenated elements

[Florian Westphal]

Currently there is no case where the individual expressions inside a
mapped concatenation need to be munged.

However, to support proper delinearization for an input like
'rule netdev nt nc set update ether saddr . vlan id timeout 5s @macset'

we need to allow this.

Right now, this gets listed as:

update @macset { @ll,48,48 . @ll,112,16 & 0xfff timeout 5s }

because the ethernet protocol is replaced by vlan beforehand,
so we fail to map @ll,48,48 to a vlan protocol.

Likewise, we can't map the vlan info either because we cannot
cope with the 'and' operation properly, nor is it removed.

Prepare for this by deleting and re-adding so that we do not
corrupt the linked list.

After this, the list can be safely changed and a followup patch
can start to delete/reallocate expressions.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes
 src/netlink_delinearize.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 3bdd98d47eb0..3835b3e522b9 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2539,16 +2539,21 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		unsigned int type = expr->dtype->type, ntype = 0;
 		int off = expr->dtype->subtypes;
 		const struct datatype *dtype;
+		LIST_HEAD(tmp);
+		struct expr *n;
 
-		list_for_each_entry(i, &expr->expressions, list) {
+		list_for_each_entry_safe(i, n, &expr->expressions, list) {
 			if (type) {
 				dtype = concat_subtype_lookup(type, --off);
 				expr_set_type(i, dtype, dtype->byteorder);
 			}
+			list_del(&i->list);
 			expr_postprocess(ctx, &i);
+			list_add_tail(&i->list, &tmp);
 
 			ntype = concat_subtype_add(ntype, i->dtype->type);
 		}
+		list_splice(&tmp, &expr->expressions);
 		datatype_set(expr, concat_type_alloc(ntype));
 		break;
 	}
-- 
2.35.1

[PATCH nft v2 2/8] netlink_delinearize: postprocess binary ands in concatenations

[Florian Westphal]

Input:
update ether saddr . vlan id timeout 5s @macset
ether saddr . vlan id @macset

Before this patch, gets rendered as:
update @macset { @ll,48,48 . @ll,112,16 & 0xfff timeout 5s }
@ll,48,48 . @ll,112,16 & 0xfff @macset

After this, listing will show:
update @macset { @ll,48,48 . vlan id timeout 5s }
@ll,48,48 . vlan id @macset

The @ll, ... is due to vlan description replacing the ethernet one,
so payload decode fails to take the concatenation apart (the ethernet
header payload info is matched vs. vlan template).

This will be adjusted by a followup patch.

v2: expr_free() in __binop_postprocess needs to be moved
downwards, because expr and expr_binop are identical in the
OP_AND case. (Pablo).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: amend it to also handle 'set elem keys', not just
     concatenations.  Update comment and commit log.
 include/netlink.h         |  6 ++++++
 src/netlink_delinearize.c | 45 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/include/netlink.h b/include/netlink.h
index e8e0f68ae1a4..71c888fa0b40 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -42,10 +42,16 @@ struct netlink_parse_ctx {
 	struct netlink_ctx	*nlctx;
 };
 
+
+#define RULE_PP_IN_CONCATENATION	(1 << 0)
+
+#define RULE_PP_REMOVE_OP_AND		(RULE_PP_IN_CONCATENATION)
+
 struct rule_pp_ctx {
 	struct proto_ctx	pctx;
 	struct payload_dep_ctx	pdctx;
 	struct stmt		*stmt;
+	unsigned int		flags;
 };
 
 extern const struct input_descriptor indesc_netlink;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 3835b3e522b9..42221f8ca526 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2260,12 +2260,13 @@ static void binop_adjust(const struct expr *binop, struct expr *right,
 	}
 }
 
-static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr,
-			      struct expr **expr_binop)
+static void __binop_postprocess(struct rule_pp_ctx *ctx,
+				struct expr *expr,
+				struct expr *left,
+				struct expr *mask,
+				struct expr **expr_binop)
 {
 	struct expr *binop = *expr_binop;
-	struct expr *left = binop->left;
-	struct expr *mask = binop->right;
 	unsigned int shift;
 
 	assert(binop->etype == EXPR_BINOP);
@@ -2301,15 +2302,26 @@ static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr,
 
 		assert(binop->left == left);
 		*expr_binop = expr_get(left);
-		expr_free(binop);
 
 		if (left->etype == EXPR_PAYLOAD)
 			payload_match_postprocess(ctx, expr, left);
 		else if (left->etype == EXPR_EXTHDR && right)
 			expr_set_type(right, left->dtype, left->byteorder);
+
+		expr_free(binop);
 	}
 }
 
+static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr,
+			      struct expr **expr_binop)
+{
+	struct expr *binop = *expr_binop;
+	struct expr *left = binop->left;
+	struct expr *mask = binop->right;
+
+	__binop_postprocess(ctx, expr, left, mask, expr_binop);
+}
+
 static void map_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 {
 	struct expr *binop = expr->map;
@@ -2542,6 +2554,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		LIST_HEAD(tmp);
 		struct expr *n;
 
+		ctx->flags |= RULE_PP_IN_CONCATENATION;
 		list_for_each_entry_safe(i, n, &expr->expressions, list) {
 			if (type) {
 				dtype = concat_subtype_lookup(type, --off);
@@ -2553,6 +2566,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 
 			ntype = concat_subtype_add(ntype, i->dtype->type);
 		}
+		ctx->flags &= ~RULE_PP_IN_CONCATENATION;
 		list_splice(&tmp, &expr->expressions);
 		datatype_set(expr, concat_type_alloc(ntype));
 		break;
@@ -2569,6 +2583,27 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 			expr_set_type(expr->right, &integer_type,
 				      BYTEORDER_HOST_ENDIAN);
 			break;
+		case OP_AND:
+			expr_set_type(expr->right, expr->left->dtype,
+				      expr->left->byteorder);
+
+			/* Do not process OP_AND in ordinary rule context.
+			 *
+			 * Removal needs to be performed as part of the relational
+			 * operation because the RHS constant might need to be adjusted
+			 * (shifted).
+			 *
+			 * This is different in set element context or concatenations:
+			 * There is no relational operation (eq, neq and so on), thus
+			 * it needs to be processed right away.
+			 */
+			if ((ctx->flags & RULE_PP_REMOVE_OP_AND) &&
+			    expr->left->etype == EXPR_PAYLOAD &&
+			    expr->right->etype == EXPR_VALUE) {
+				__binop_postprocess(ctx, expr, expr->left, expr->right, exprp);
+				return;
+			}
+			break;
 		default:
 			expr_set_type(expr->right, expr->left->dtype,
 				      expr->left->byteorder);
-- 
2.35.1

[PATCH nft v2 3/8] proto: track full stack of seen l2 protocols, not just cumulative offset

[Florian Westphal]

For input, a cumulative size counter of all pushed l2 headers is enough,
because we have the full expression tree available to us.

For delinearization we need to track all seen l2 headers, else we lose
information that we might need at a later time.

Consider:

rule netdev nt nc set update ether saddr . vlan id

during delinearization, the vlan proto_desc replaces the ethernet one,
and by the time we try to split the concatenation apart we will search
the ether saddr offset vs. the templates for proto_vlan.

This replaces the offset with an array that stores the protocol
descriptions seen.

Then, if the payload offset is larger than our description, search the
l2 stack and adjust the offset until we're within the expected offset
boundary.

Reported-by: Eric Garver <eric@garver.life>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes.

 include/proto.h           |  3 +-
 src/evaluate.c            | 15 +++++++--
 src/netlink_delinearize.c |  5 ---
 src/payload.c             | 67 ++++++++++++++++++++++++++++++++-------
 src/proto.c               |  2 --
 5 files changed, 71 insertions(+), 21 deletions(-)

diff --git a/include/proto.h b/include/proto.h
index a04240a5de81..35e760c7e16e 100644
--- a/include/proto.h
+++ b/include/proto.h
@@ -193,13 +193,14 @@ struct proto_ctx {
 	struct {
 		struct location			location;
 		const struct proto_desc		*desc;
-		unsigned int			offset;
 		struct {
 			struct location		location;
 			const struct proto_desc	*desc;
 		} protos[PROTO_CTX_NUM_PROTOS];
 		unsigned int			num_protos;
 	} protocol[PROTO_BASE_MAX + 1];
+	const struct proto_desc *stacked_ll[PROTO_CTX_NUM_PROTOS];
+	uint8_t stacked_ll_count;
 };
 
 extern void proto_ctx_init(struct proto_ctx *ctx, unsigned int family,
diff --git a/src/evaluate.c b/src/evaluate.c
index 9ae525769bc3..be9fcd5117fb 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -678,7 +678,13 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 	    conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
 		return 1;
 
-	payload->payload.offset += ctx->pctx.protocol[base].offset;
+	if (base == PROTO_BASE_LL_HDR) {
+		unsigned int i;
+
+		for (i = 0; i < ctx->pctx.stacked_ll_count; i++)
+			payload->payload.offset += ctx->pctx.stacked_ll[i]->length;
+	}
+
 	rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 
 	return 0;
@@ -727,7 +733,12 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 	if (desc == payload->payload.desc) {
 		const struct proto_hdr_template *tmpl;
 
-		payload->payload.offset += ctx->pctx.protocol[base].offset;
+		if (desc->base == PROTO_BASE_LL_HDR) {
+			unsigned int i;
+
+			for (i = 0; i < ctx->pctx.stacked_ll_count; i++)
+				payload->payload.offset += ctx->pctx.stacked_ll[i]->length;
+		}
 check_icmp:
 		if (desc != &proto_icmp && desc != &proto_icmp6)
 			return 0;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 42221f8ca526..8851043bf277 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1977,11 +1977,6 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
 				      struct expr *expr,
 				      struct expr *payload)
 {
-	enum proto_bases base = payload->payload.base;
-
-	assert(payload->payload.offset >= ctx->pctx.protocol[base].offset);
-	payload->payload.offset -= ctx->pctx.protocol[base].offset;
-
 	switch (expr->op) {
 	case OP_EQ:
 	case OP_NEQ:
diff --git a/src/payload.c b/src/payload.c
index 66418cddb3b5..2c0d0ac9e8ae 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -116,8 +116,13 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
 	if (desc->base == base->base) {
 		assert(base->length > 0);
 
-		if (!left->payload.is_raw)
-			ctx->protocol[base->base].offset += base->length;
+		if (!left->payload.is_raw) {
+			if (desc->base == PROTO_BASE_LL_HDR &&
+			    ctx->stacked_ll_count < PROTO_CTX_NUM_PROTOS) {
+				ctx->stacked_ll[ctx->stacked_ll_count] = base;
+				ctx->stacked_ll_count++;
+			}
+		}
 	}
 	proto_ctx_update(ctx, desc->base, loc, desc);
 }
@@ -869,6 +874,38 @@ void exthdr_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
 	}
 }
 
+static const struct proto_desc *get_stacked_desc(const struct proto_ctx *ctx,
+						 const struct proto_desc *top,
+						 const struct expr *e,
+						 unsigned int *skip)
+{
+	unsigned int i, total, payload_offset = e->payload.offset;
+
+	assert(e->etype == EXPR_PAYLOAD);
+
+	if (e->payload.base != PROTO_BASE_LL_HDR ||
+	    payload_offset < top->length) {
+		*skip = 0;
+		return top;
+	}
+
+	for (i = 0, total = 0; i < ctx->stacked_ll_count; i++) {
+		const struct proto_desc *stacked;
+
+		stacked = ctx->stacked_ll[i];
+		if (payload_offset < stacked->length) {
+			*skip = total;
+			return stacked;
+		}
+
+		payload_offset -= stacked->length;
+		total += stacked->length;
+	}
+
+	*skip = total;
+	return top;
+}
+
 /**
  * payload_expr_complete - fill in type information of a raw payload expr
  *
@@ -880,9 +917,10 @@ void exthdr_dependency_kill(struct payload_dep_ctx *ctx, struct expr *expr,
  */
 void payload_expr_complete(struct expr *expr, const struct proto_ctx *ctx)
 {
+	unsigned int payload_offset = expr->payload.offset;
 	const struct proto_desc *desc;
 	const struct proto_hdr_template *tmpl;
-	unsigned int i;
+	unsigned int i, total;
 
 	assert(expr->etype == EXPR_PAYLOAD);
 
@@ -891,9 +929,12 @@ void payload_expr_complete(struct expr *expr, const struct proto_ctx *ctx)
 		return;
 	assert(desc->base == expr->payload.base);
 
+	desc = get_stacked_desc(ctx, desc, expr, &total);
+	payload_offset -= total;
+
 	for (i = 0; i < array_size(desc->templates); i++) {
 		tmpl = &desc->templates[i];
-		if (tmpl->offset != expr->payload.offset ||
+		if (tmpl->offset != payload_offset ||
 		    tmpl->len    != expr->len)
 			continue;
 
@@ -950,6 +991,7 @@ bool payload_expr_trim(struct expr *expr, struct expr *mask,
 	unsigned int payload_len = expr->len;
 	const struct proto_desc *desc;
 	unsigned int off, i, len = 0;
+	unsigned int total;
 
 	assert(expr->etype == EXPR_PAYLOAD);
 
@@ -959,10 +1001,8 @@ bool payload_expr_trim(struct expr *expr, struct expr *mask,
 
 	assert(desc->base == expr->payload.base);
 
-	if (ctx->protocol[expr->payload.base].offset) {
-		assert(payload_offset >= ctx->protocol[expr->payload.base].offset);
-		payload_offset -= ctx->protocol[expr->payload.base].offset;
-	}
+	desc = get_stacked_desc(ctx, desc, expr, &total);
+	payload_offset -= total;
 
 	off = round_up(mask->len, BITS_PER_BYTE) - mask_len;
 	payload_offset += off;
@@ -1009,10 +1049,11 @@ bool payload_expr_trim(struct expr *expr, struct expr *mask,
 void payload_expr_expand(struct list_head *list, struct expr *expr,
 			 const struct proto_ctx *ctx)
 {
+	unsigned int payload_offset = expr->payload.offset;
 	const struct proto_hdr_template *tmpl;
 	const struct proto_desc *desc;
+	unsigned int i, total;
 	struct expr *new;
-	unsigned int i;
 
 	assert(expr->etype == EXPR_PAYLOAD);
 
@@ -1021,13 +1062,16 @@ void payload_expr_expand(struct list_head *list, struct expr *expr,
 		goto raw;
 	assert(desc->base == expr->payload.base);
 
+	desc = get_stacked_desc(ctx, desc, expr, &total);
+	payload_offset -= total;
+
 	for (i = 1; i < array_size(desc->templates); i++) {
 		tmpl = &desc->templates[i];
 
 		if (tmpl->len == 0)
 			break;
 
-		if (tmpl->offset != expr->payload.offset)
+		if (tmpl->offset != payload_offset)
 			continue;
 
 		if (tmpl->icmp_dep && ctx->th_dep.icmp.type &&
@@ -1039,6 +1083,7 @@ void payload_expr_expand(struct list_head *list, struct expr *expr,
 			list_add_tail(&new->list, list);
 			expr->len	     -= tmpl->len;
 			expr->payload.offset += tmpl->len;
+			payload_offset       += tmpl->len;
 			if (expr->len == 0)
 				return;
 		} else if (expr->len > 0) {
@@ -1051,7 +1096,7 @@ void payload_expr_expand(struct list_head *list, struct expr *expr,
 	}
 raw:
 	new = payload_expr_alloc(&expr->location, NULL, 0);
-	payload_init_raw(new, expr->payload.base, expr->payload.offset,
+	payload_init_raw(new, expr->payload.base, payload_offset,
 			 expr->len);
 	list_add_tail(&new->list, list);
 }
diff --git a/src/proto.c b/src/proto.c
index a013a00d2c7b..2663f216860b 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -160,8 +160,6 @@ static void proto_ctx_debug(const struct proto_ctx *ctx, enum proto_bases base,
 			 proto_base_names[i],
 			 ctx->protocol[i].desc ? ctx->protocol[i].desc->name :
 						 "none");
-		if (ctx->protocol[i].offset)
-			pr_debug(" (offset: %u)", ctx->protocol[i].offset);
 		if (i == base)
 			pr_debug(" <-");
 		pr_debug("\n");
-- 
2.35.1

[PATCH nft v2 4/8] debug: dump the l2 protocol stack

[Florian Westphal]

Previously we used to print the cumulative size of the headers,
update this to print the tracked l2 stack.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes.
 src/proto.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/proto.c b/src/proto.c
index 2663f216860b..c49648275e12 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -154,6 +154,12 @@ static void proto_ctx_debug(const struct proto_ctx *ctx, enum proto_bases base,
 	if (!(debug_mask & NFT_DEBUG_PROTO_CTX))
 		return;
 
+	if (base == PROTO_BASE_LL_HDR && ctx->stacked_ll_count) {
+		pr_debug(" saved ll headers:");
+		for (i = 0; i < ctx->stacked_ll_count; i++)
+			pr_debug(" %s", ctx->stacked_ll[i]->name);
+	}
+
 	pr_debug("update %s protocol context:\n", proto_base_names[base]);
 	for (i = PROTO_BASE_LL_HDR; i <= PROTO_BASE_MAX; i++) {
 		pr_debug(" %-20s: %s",
-- 
2.35.1

[PATCH nft v2 5/8] tests: add a test case for ether and vlan listing

[Florian Westphal]

before this patch series, test fails dump validation:
-               update @macset { ether saddr . vlan id timeout 5s } counter packets 0 bytes 0
-               ether saddr . vlan id @macset
+               update @macset { @ll,48,48 . @ll,112,16 & 0xfff timeout 5s } counter packets 0 bytes 0
+               @ll,48,48 . @ll,112,16 & 0xfff @macset

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes.

 tests/shell/testcases/sets/0070stacked_l2_headers  |  6 ++++++
 .../sets/dumps/0070stacked_l2_headers.nft          | 14 ++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100755 tests/shell/testcases/sets/0070stacked_l2_headers
 create mode 100644 tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft

diff --git a/tests/shell/testcases/sets/0070stacked_l2_headers b/tests/shell/testcases/sets/0070stacked_l2_headers
new file mode 100755
index 000000000000..07820b7c4fdd
--- /dev/null
+++ b/tests/shell/testcases/sets/0070stacked_l2_headers
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+set -e
+dumpfile=$(dirname $0)/dumps/$(basename $0).nft
+
+$NFT -f "$dumpfile"
diff --git a/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft b/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft
new file mode 100644
index 000000000000..ef254b96879e
--- /dev/null
+++ b/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft
@@ -0,0 +1,14 @@
+table netdev nt {
+	set macset {
+		typeof ether saddr . vlan id
+		size 1024
+		flags dynamic,timeout
+	}
+
+	chain nc {
+		update @macset { ether saddr . vlan id timeout 5s } counter packets 0 bytes 0
+		ether saddr . vlan id @macset
+		vlan pcp 1
+		ether saddr 0a:0b:0c:0d:0e:0f vlan id 42
+	}
+}
-- 
2.35.1

[PATCH nft v2 6/8] netlink_delinearize: also postprocess OP_AND in set element context

[Florian Westphal]

Pablo reports:
add rule netdev nt y update @macset { vlan id timeout 5s }

listing still shows the raw expression:
 update @macset { @ll,112,16 & 0xfff timeout 5s }

so also cover the 'set element' case.

Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: new.

 include/netlink.h                                  |  4 +++-
 src/netlink_delinearize.c                          |  2 ++
 .../sets/dumps/0070stacked_l2_headers.nft          | 14 ++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/netlink.h b/include/netlink.h
index 71c888fa0b40..63d07edf419e 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -44,8 +44,10 @@ struct netlink_parse_ctx {
 
 
 #define RULE_PP_IN_CONCATENATION	(1 << 0)
+#define RULE_PP_IN_SET_ELEM		(1 << 1)
 
-#define RULE_PP_REMOVE_OP_AND		(RULE_PP_IN_CONCATENATION)
+#define RULE_PP_REMOVE_OP_AND		(RULE_PP_IN_CONCATENATION | \
+					 RULE_PP_IN_SET_ELEM)
 
 struct rule_pp_ctx {
 	struct proto_ctx	pctx;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 8851043bf277..0da6cc78f94f 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2661,7 +2661,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		expr_postprocess(ctx, &expr->prefix);
 		break;
 	case EXPR_SET_ELEM:
+		ctx->flags |= RULE_PP_IN_SET_ELEM;
 		expr_postprocess(ctx, &expr->key);
+		ctx->flags &= ~RULE_PP_IN_SET_ELEM;
 		break;
 	case EXPR_EXTHDR:
 		exthdr_dependency_kill(&ctx->pdctx, expr, ctx->pctx.family);
diff --git a/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft b/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft
index ef254b96879e..0057e9c62e4d 100644
--- a/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft
+++ b/tests/shell/testcases/sets/dumps/0070stacked_l2_headers.nft
@@ -1,14 +1,28 @@
 table netdev nt {
+	set vlanidset {
+		typeof vlan id
+		size 1024
+		flags dynamic,timeout
+	}
+
 	set macset {
 		typeof ether saddr . vlan id
 		size 1024
 		flags dynamic,timeout
 	}
 
+	set ipset {
+		typeof vlan id . ip saddr
+		size 1024
+		flags dynamic,timeout
+	}
+
 	chain nc {
 		update @macset { ether saddr . vlan id timeout 5s } counter packets 0 bytes 0
 		ether saddr . vlan id @macset
 		vlan pcp 1
 		ether saddr 0a:0b:0c:0d:0e:0f vlan id 42
+		update @vlanidset { vlan id timeout 5s } counter packets 0 bytes 0
+		update @ipset { vlan id . ip saddr timeout 5s } counter packets 0 bytes 0
 	}
 }
-- 
2.35.1

[PATCH nft v2 7/8] evaluate: search stacked header list for matching payload dep

[Florian Westphal]

"ether saddr 0:1:2:3:4:6 vlan id 2" works, but reverse fails:

"vlan id 2 ether saddr 0:1:2:3:4:6" will give
Error: conflicting protocols specified: vlan vs. ether

After "proto: track full stack of seen l2 protocols, not just cumulative offset",
we have a list of all l2 headers, so search those to see if we had this
proto base in the past before rejecting this.

Reported-by: Eric Garver <eric@garver.life>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes.

 src/evaluate.c                        | 21 +++++++---
 tests/py/bridge/vlan.t                |  3 ++
 tests/py/bridge/vlan.t.json           | 56 +++++++++++++++++++++++++++
 tests/py/bridge/vlan.t.payload        | 16 ++++++++
 tests/py/bridge/vlan.t.payload.netdev | 20 ++++++++++
 5 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index be9fcd5117fb..919c38c5604e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -659,13 +659,22 @@ static int resolve_protocol_conflict(struct eval_ctx *ctx,
 	struct stmt *nstmt = NULL;
 	int link, err;
 
-	if (payload->payload.base == PROTO_BASE_LL_HDR &&
-	    proto_is_dummy(desc)) {
-		err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
-		if (err < 0)
-			return err;
+	if (payload->payload.base == PROTO_BASE_LL_HDR) {
+		if (proto_is_dummy(desc)) {
+			err = meta_iiftype_gen_dependency(ctx, payload, &nstmt);
+			if (err < 0)
+				return err;
 
-		rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+			rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+		} else {
+			unsigned int i;
+
+			/* payload desc stored in the L2 header stack? No conflict. */
+			for (i = 0; i < ctx->pctx.stacked_ll_count; i++) {
+				if (ctx->pctx.stacked_ll[i] == payload->payload.desc)
+					return 0;
+			}
+		}
 	}
 
 	assert(base <= PROTO_BASE_MAX);
diff --git a/tests/py/bridge/vlan.t b/tests/py/bridge/vlan.t
index 924ed4ed3679..49206017fff2 100644
--- a/tests/py/bridge/vlan.t
+++ b/tests/py/bridge/vlan.t
@@ -47,3 +47,6 @@ ether type ip vlan id 1 ip saddr 10.0.0.1;fail
 
 # mangling
 vlan id 1 vlan id set 2;ok
+
+ether saddr 00:01:02:03:04:05 vlan id 1;ok
+vlan id 2 ether saddr 0:1:2:3:4:6;ok;ether saddr 00:01:02:03:04:06 vlan id 2
diff --git a/tests/py/bridge/vlan.t.json b/tests/py/bridge/vlan.t.json
index e7640f9a6a37..58d4a40f5baf 100644
--- a/tests/py/bridge/vlan.t.json
+++ b/tests/py/bridge/vlan.t.json
@@ -761,3 +761,59 @@
         }
     }
 ]
+
+# ether saddr 00:01:02:03:04:05 vlan id 1
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr",
+                    "protocol": "ether"
+                }
+            },
+            "op": "==",
+            "right": "00:01:02:03:04:05"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    }
+]
+
+# vlan id 2 ether saddr 0:1:2:3:4:6
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "saddr",
+                    "protocol": "ether"
+                }
+            },
+            "op": "==",
+            "right": "00:01:02:03:04:06"
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "vlan"
+                }
+            },
+            "op": "==",
+            "right": 2
+        }
+    }
+]
diff --git a/tests/py/bridge/vlan.t.payload b/tests/py/bridge/vlan.t.payload
index 6c8d595a1aad..713670e9e721 100644
--- a/tests/py/bridge/vlan.t.payload
+++ b/tests/py/bridge/vlan.t.payload
@@ -276,3 +276,19 @@ bridge
   [ payload load 2b @ link header + 14 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ]
   [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
+# ether saddr 00:01:02:03:04:05 vlan id 1
+bridge test-bridge input
+  [ payload load 8b @ link header + 6 => reg 1 ]
+  [ cmp eq reg 1 0x03020100 0x00810504 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
+
+# vlan id 2 ether saddr 0:1:2:3:4:6
+bridge test-bridge input
+  [ payload load 8b @ link header + 6 => reg 1 ]
+  [ cmp eq reg 1 0x03020100 0x00810604 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000200 ]
diff --git a/tests/py/bridge/vlan.t.payload.netdev b/tests/py/bridge/vlan.t.payload.netdev
index d2c7d74a4e85..98a2a2b0f379 100644
--- a/tests/py/bridge/vlan.t.payload.netdev
+++ b/tests/py/bridge/vlan.t.payload.netdev
@@ -322,3 +322,23 @@ netdev
   [ payload load 2b @ link header + 14 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ]
   [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
+# vlan id 2 ether saddr 0:1:2:3:4:6
+netdev test-netdev ingress
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 8b @ link header + 6 => reg 1 ]
+  [ cmp eq reg 1 0x03020100 0x00810604 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000200 ]
+
+# ether saddr 00:01:02:03:04:05 vlan id 1
+netdev test-netdev ingress
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 8b @ link header + 6 => reg 1 ]
+  [ cmp eq reg 1 0x03020100 0x00810504 ]
+  [ payload load 2b @ link header + 14 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ cmp eq reg 1 0x00000100 ]
-- 
2.35.1

[PATCH nft v2 8/8] src: allow anon set concatenation with ether and vlan

[Florian Westphal]

vlan id uses integer type (which has a length of 0).

Using it was possible, but listing would assert:
python: mergesort.c:24: concat_expr_msort_value: Assertion `ilen > 0' failed.

There are two reasons for this.
First reason is that the udata/typeof information lacks the 'vlan id'
part, because internally this is 'payload . binop(payload AND mask)'.

binop lacks an udata store.  It makes little sense to store it,
'typeof' keyword expects normal match syntax.

So, when storing udata, store the left hand side of the binary
operation, i.e. the load of the 2-byte key.

With that resolved, delinerization could work, but concat_elem_expr()
would splice 12 bits off the elements value, but it should be 16 (on
a byte boundary).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: no changes.

 src/expression.c                      | 17 +++++++++--
 src/netlink.c                         | 10 +++++--
 tests/py/bridge/vlan.t                |  2 ++
 tests/py/bridge/vlan.t.json           | 41 +++++++++++++++++++++++++++
 tests/py/bridge/vlan.t.payload        | 12 ++++++++
 tests/py/bridge/vlan.t.payload.netdev | 14 +++++++++
 6 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index deb649e1847b..7390089cf57d 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -879,17 +879,30 @@ static void concat_expr_print(const struct expr *expr, struct output_ctx *octx)
 #define NFTNL_UDATA_SET_KEY_CONCAT_SUB_DATA 1
 #define NFTNL_UDATA_SET_KEY_CONCAT_SUB_MAX  2
 
+static struct expr *expr_build_udata_recurse(struct expr *e)
+{
+	switch (e->etype) {
+	case EXPR_BINOP:
+		return e->left;
+	default:
+		break;
+	}
+
+	return e;
+}
+
 static int concat_expr_build_udata(struct nftnl_udata_buf *udbuf,
 				    const struct expr *concat_expr)
 {
 	struct nftnl_udata *nest;
+	struct expr *expr, *tmp;
 	unsigned int i = 0;
-	struct expr *expr;
 
-	list_for_each_entry(expr, &concat_expr->expressions, list) {
+	list_for_each_entry_safe(expr, tmp, &concat_expr->expressions, list) {
 		struct nftnl_udata *nest_expr;
 		int err;
 
+		expr = expr_build_udata_recurse(expr);
 		if (!expr_ops(expr)->build_udata || i >= NFT_REG32_SIZE)
 			return -1;
 
diff --git a/src/netlink.c b/src/netlink.c
index 89d864ed046a..799cf9b8ebef 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1114,17 +1114,21 @@ static struct expr *concat_elem_expr(struct expr *key,
 				     struct expr *data, int *off)
 {
 	const struct datatype *subtype;
+	unsigned int sub_length;
 	struct expr *expr;
 
 	if (key) {
 		(*off)--;
-		expr = constant_expr_splice(data, key->len);
+		sub_length = round_up(key->len, BITS_PER_BYTE);
+
+		expr = constant_expr_splice(data, sub_length);
 		expr->dtype = datatype_get(key->dtype);
 		expr->byteorder = key->byteorder;
 		expr->len = key->len;
 	} else {
 		subtype = concat_subtype_lookup(dtype->type, --(*off));
-		expr = constant_expr_splice(data, subtype->size);
+		sub_length = round_up(subtype->size, BITS_PER_BYTE);
+		expr = constant_expr_splice(data, sub_length);
 		expr->dtype = subtype;
 		expr->byteorder = subtype->byteorder;
 	}
@@ -1136,7 +1140,7 @@ static struct expr *concat_elem_expr(struct expr *key,
 	    expr->dtype->basetype->type == TYPE_BITMASK)
 		expr = bitmask_expr_to_binops(expr);
 
-	data->len -= netlink_padding_len(expr->len);
+	data->len -= netlink_padding_len(sub_length);
 
 	return expr;
 }
diff --git a/tests/py/bridge/vlan.t b/tests/py/bridge/vlan.t
index 49206017fff2..95bdff4f1b75 100644
--- a/tests/py/bridge/vlan.t
+++ b/tests/py/bridge/vlan.t
@@ -50,3 +50,5 @@ vlan id 1 vlan id set 2;ok
 
 ether saddr 00:01:02:03:04:05 vlan id 1;ok
 vlan id 2 ether saddr 0:1:2:3:4:6;ok;ether saddr 00:01:02:03:04:06 vlan id 2
+
+ether saddr . vlan id { 0a:0b:0c:0d:0e:0f . 42, 0a:0b:0c:0d:0e:0f . 4095 };ok
diff --git a/tests/py/bridge/vlan.t.json b/tests/py/bridge/vlan.t.json
index 58d4a40f5baf..f77756f5080a 100644
--- a/tests/py/bridge/vlan.t.json
+++ b/tests/py/bridge/vlan.t.json
@@ -817,3 +817,44 @@
         }
     }
 ]
+
+# ether saddr . vlan id { 0a:0b:0c:0d:0e:0f . 42, 0a:0b:0c:0d:0e:0f . 4095 }
+[
+    {
+        "match": {
+            "left": {
+                "concat": [
+                    {
+                        "payload": {
+                            "field": "saddr",
+                            "protocol": "ether"
+                        }
+                    },
+                    {
+                        "payload": {
+                            "field": "id",
+                            "protocol": "vlan"
+                        }
+                    }
+                ]
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    {
+                        "concat": [
+                            "0a:0b:0c:0d:0e:0f",
+                            42
+                        ]
+                    },
+                    {
+                        "concat": [
+                            "0a:0b:0c:0d:0e:0f",
+                            4095
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/bridge/vlan.t.payload b/tests/py/bridge/vlan.t.payload
index 713670e9e721..62e4b89bd0b2 100644
--- a/tests/py/bridge/vlan.t.payload
+++ b/tests/py/bridge/vlan.t.payload
@@ -292,3 +292,15 @@ bridge test-bridge input
   [ payload load 2b @ link header + 14 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
   [ cmp eq reg 1 0x00000200 ]
+
+# ether saddr . vlan id { 0a:0b:0c:0d:0e:0f . 42, 0a:0b:0c:0d:0e:0f . 4095 }
+__set%d test-bridge 3 size 2
+__set%d test-bridge 0
+	element 0d0c0b0a 00000f0e 00002a00  : 0 [end]	element 0d0c0b0a 00000f0e 0000ff0f  : 0 [end]
+bridge test-bridge input
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 6b @ link header + 6 => reg 1 ]
+  [ payload load 2b @ link header + 14 => reg 10 ]
+  [ bitwise reg 10 = ( reg 10 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ lookup reg 1 set __set%d ]
diff --git a/tests/py/bridge/vlan.t.payload.netdev b/tests/py/bridge/vlan.t.payload.netdev
index 98a2a2b0f379..1018d4c6588f 100644
--- a/tests/py/bridge/vlan.t.payload.netdev
+++ b/tests/py/bridge/vlan.t.payload.netdev
@@ -342,3 +342,17 @@ netdev test-netdev ingress
   [ payload load 2b @ link header + 14 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
   [ cmp eq reg 1 0x00000100 ]
+
+# ether saddr . vlan id { 0a:0b:0c:0d:0e:0f . 42, 0a:0b:0c:0d:0e:0f . 4095 }
+__set%d test-netdev 3 size 2
+__set%d test-netdev 0
+	element 0d0c0b0a 00000f0e 00002a00  : 0 [end]	element 0d0c0b0a 00000f0e 0000ff0f  : 0 [end]
+netdev test-netdev ingress
+  [ meta load iiftype => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ link header + 12 => reg 1 ]
+  [ cmp eq reg 1 0x00000081 ]
+  [ payload load 6b @ link header + 6 => reg 1 ]
+  [ payload load 2b @ link header + 14 => reg 10 ]
+  [ bitwise reg 10 = ( reg 10 & 0x0000ff0f ) ^ 0x00000000 ]
+  [ lookup reg 1 set __set%d ]
-- 
2.35.1

Re: [PATCH nft v2 0/8] really handle stacked l2 headers

[Pablo Neira Ayuso]

Hi Florian,

On Mon, Aug 01, 2022 at 03:56:25PM +0200, Florian Westphal wrote:
> v2:
> - fix a UAF during rule listing.  When OP_AND gets culled,
>   'expr' is free'd as well ahead of time because they alias one
>   another in the set key case (there is no compare/relational op).
> - add and handle plain 'vlan id' in a set key.
>   in v1, this would be shown with the '& 0xfff' included, because
>   v1 only removed OP_AND in concatenations.
> 
> Eric Garver reported a number of issues when matching vlan headers:
> 
> In:  update @macset { ether saddr . vlan id timeout 5s }
> Out: update @macset { @ll,48,48 . @ll,112,16 & 0xfff timeout 5s }
> 
> This is because of amnesia in nft during expression decoding:
> When we encounter 'vlan id', the L2 protocl (ethernet) is replaced by
> vlan, so we attempt to match @ll,48,48 vs. the vlan header and come up
> empty.
> 
> The vlan decode fails because we can't handle '& 0xfff' in this
> instance, so we can locate the right offset but the payload expression
> length doesn't match the template length (16 vs 12 bits).
> 
> 
> The main patch is patch 3, which adds a stack of l2 protocols to track
> instead of only keeping the cumulative size.
> 
> The latter is ok for serialization (we have the expression tree, so its
> enough to add the size of the 'previous' l2 headers to payload
> expressions that match the new 'top' l2 header.
> 
> But for deserialization, we need to be able to search all protocols base
> headers seen.
> 
> The remaining patches improve handling of 'integer base type'
> expressions and add test cases.

series LGTM.

A few more nits:

# cat test.nft
add table netdev x
add chain netdev x y
add rule netdev x y ip saddr 1.2.3.4 vlan id 10
# nft -f test.nft
test.nft:3:38-44: Error: conflicting protocols specified: ether vs. vlan
add rule netdev x y ip saddr 1.2.3.4 vlan id 10
                                     ^^^^^^^

it also occurs here:

# cat test.nft
add table netdev x
add chain netdev x y
add set netdev x macset { typeof ip saddr . vlan id; flags dynamic,timeout; }
add rule netdev x y update @macset { ip saddr . vlan id }
# nft -f test.nft
test.nft:4:49-55: Error: conflicting protocols specified: ether vs. vlan
add rule netdev x y update @macset { ip saddr . vlan id }
                                                ^^^^^^^

This is related to an implicit ether dependency.

If you see a way to fix this incrementally, I'm fine with you pushing
out this series and then you follow up.

Another issue: probably it would make sense to bail out when trying to
use 'vlan id' (and any other vlan fields) from ip/ip6/inet families?
vlan_do_receive() sets skb->dev to the vlan device, and the vlan
fields in the skbuff are cleared. In iptables, there is not vlan match
for this reason.

Re: [PATCH nft v2 0/8] really handle stacked l2 headers

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> series LGTM.
> 
> A few more nits:
> 
> # cat test.nft
> add table netdev x
> add chain netdev x y
> add rule netdev x y ip saddr 1.2.3.4 vlan id 10
> # nft -f test.nft
> test.nft:3:38-44: Error: conflicting protocols specified: ether vs. vlan
> add rule netdev x y ip saddr 1.2.3.4 vlan id 10
>                                      ^^^^^^^

But thats not a regression, right?

> # cat test.nft
> add table netdev x
> add chain netdev x y
> add set netdev x macset { typeof ip saddr . vlan id; flags dynamic,timeout; }
> add rule netdev x y update @macset { ip saddr . vlan id }
> # nft -f test.nft
> test.nft:4:49-55: Error: conflicting protocols specified: ether vs. vlan
> add rule netdev x y update @macset { ip saddr . vlan id }
>                                                 ^^^^^^^
> 
> This is related to an implicit ether dependency.

Yes, it needs two implcit deps.

> If you see a way to fix this incrementally, I'm fine with you pushing
> out this series and then you follow up.

OK, will do that then.

> Another issue: probably it would make sense to bail out when trying to
> use 'vlan id' (and any other vlan fields) from ip/ip6/inet families?
> vlan_do_receive() sets skb->dev to the vlan device, and the vlan
> fields in the skbuff are cleared. In iptables, there is not vlan match
> for this reason.

Thanks for the hint.  Right, so it makes sense to refuse the implcit dep
and/or reject it from eval phase.

I will have a look next week.