[PATCH nft 1/3] segtree: assert on value expressions

[PATCH nft 1/3] segtree: assert on value expressions

[Pablo Neira Ayuso]

Assert value expression before performing math on them instead of
crashing with unsupported expressions.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/segtree.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/segtree.c b/src/segtree.c
index c8a7a3541ac3..4f758e6c9f99 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -253,6 +253,9 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 		if (i->key->flags & EXPR_F_INTERVAL_END && left) {
 			list_del(&left->list);
 			list_del(&i->list);
+
+			assert(i->key->etype == EXPR_VALUE);
+
 			mpz_sub_ui(i->key->value, i->key->value, 1);
 			range = get_set_interval_find(cache_set, left, i);
 			if (!range) {
@@ -325,6 +328,10 @@ static int expr_value_cmp(const void *p1, const void *p2)
 		return -1;
 
 	key_e2 = expr_value(e2);
+
+	assert(key_e1->etype == EXPR_VALUE);
+	assert(key_e2->etype == EXPR_VALUE);
+
 	ret = mpz_cmp(key_e1->value, key_e2->value);
 	if (ret == 0) {
 		if (e1->key->flags & EXPR_F_INTERVAL_END)
@@ -592,6 +599,10 @@ add_interval(struct expr *set, struct expr *low, struct expr *i, bool closed)
 	mpz_init(p);
 
 	key = expr_value(low);
+
+	assert(expr_value(i)->etype == EXPR_VALUE);
+	assert(key->etype == EXPR_VALUE);
+
 	mpz_sub(range, expr_value(i)->value, key->value);
 	if (closed)
 		mpz_sub_ui(range, range, 1);
-- 
2.47.3

[PATCH nft 2/3] Revert "segtree: basic support for binary operations in concatenated set ranges"

[Pablo Neira Ayuso]

Handling binary expression from the interval set postprocessing step is
tricky, it is easier to handle set element key as value at that stage.
Revert 1de3568ef07f ("segtree: basic support for binary operations in
concatenated set ranges"), this only works for concatenated set ranges.

A follow up patch postpones the conversion from value to symbol for
TYPE_BITMASK expressions after the interval set postprocessing.

Fixes: 1de3568ef07f ("segtree: basic support for binary operations in concatenated set ranges")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/segtree.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index 4f758e6c9f99..5e178fa3c48d 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -369,29 +369,6 @@ static int range_mask_len(const mpz_t start, const mpz_t end, unsigned int len)
 	return ret;
 }
 
-static int concat_expr_cmp(const struct expr *r1, const struct expr *r2)
-{
-	int ret;
-
-	assert(r1->etype == r2->etype);
-
-	switch (r1->etype) {
-	case EXPR_BINOP:
-		assert(r1->op == r2->op);
-		ret = 0;
-		ret = mpz_cmp(r1->left->value, r2->left->value);
-		ret |= mpz_cmp(r1->right->value, r2->right->value);
-		break;
-	case EXPR_VALUE:
-		ret = mpz_cmp(r1->value, r2->value);
-		break;
-	default:
-		BUG("unexpected expression %s", expr_name(r1->key));
-	}
-
-	return ret;
-}
-
 /* Given a set with two elements (start and end), transform them into a
  * concatenation of ranges. That is, from a list of start expressions and a list
  * of end expressions, form a list of start - end expressions.
@@ -431,13 +408,13 @@ void concat_range_aggregate(struct expr *set)
 			r2_next = r2->list.next;
 			free_r1 = 0;
 
-			if (!concat_expr_cmp(r1, r2)) {
+			assert(r1->etype == EXPR_VALUE && r1->etype == EXPR_VALUE);
+
+			if (!mpz_cmp(r1->value, r2->value)) {
 				free_r1 = 1;
 				goto next;
 			}
 
-			assert(r1->etype == EXPR_VALUE && r1->etype == EXPR_VALUE);
-
 			if (expr_basetype(r1)->type == TYPE_STRING &&
 			    expr_basetype(r2)->type == TYPE_STRING) {
 				string_type = true;
-- 
2.47.3

[PATCH nft 3/3] segtree: postpone bitmask to symbol conversion for interval sets

[Pablo Neira Ayuso]

netlink_delinearize_setelem() calls bitmask_expr_to_binops() to turn a
bitmask into symbols, ie.

	tcp flags { 0x2, 0x12 }

into
	tcp flags { syn, syn | ack }

However, this call comes too early for interval set with one single key.
The interval_map_decompose() postprocessing can only deal with value
expressions, otherwise it hits an assertion.

Move this conversion at a later stage of interval_map_decompose() for
interval sets.

Ranges and prefix do not call bitmask_expr_to_binops(), we do not use
symbolic representations for such expressions, instead values are used.

The conversion is still perform for non-interval sets, since there is no
specific function to perform postprocessing for this type of sets.

Add and extend existing tests/shell cases.

Fixes: def2cca8e713 ("set_elem: convert flag value to inclusive-OR binops during delinearize")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink.c                                 |   6 +-
 src/segtree.c                                 |  15 +
 .../dumps/vmap_concat_range_binary.json-nft   | 154 +++++++++++
 .../maps/dumps/vmap_concat_range_binary.nft   |  10 +
 .../maps/dumps/vmap_range_binary.json-nft     | 257 ++++++++++++++++++
 .../maps/dumps/vmap_range_binary.nft          |  23 ++
 .../testcases/maps/vmap_concat_range_binary   |  10 +
 tests/shell/testcases/maps/vmap_range_binary  |  35 +++
 8 files changed, 508 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/maps/dumps/vmap_range_binary.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/vmap_range_binary.nft
 create mode 100755 tests/shell/testcases/maps/vmap_range_binary

diff --git a/src/netlink.c b/src/netlink.c
index c305e5619640..8916370604c0 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1440,7 +1440,8 @@ static struct expr *concat_elem_expr(const struct set *set, struct expr *key,
 	     expr->byteorder == BYTEORDER_HOST_ENDIAN))
 		mpz_switch_byteorder(expr->value, expr->len / BITS_PER_BYTE);
 
-	if (expr->dtype->basetype != NULL &&
+	if (!(set->flags & NFT_SET_INTERVAL) &&
+	    expr->dtype->basetype != NULL &&
 	    expr->dtype->basetype->type == TYPE_BITMASK)
 		expr = bitmask_expr_to_binops(expr);
 
@@ -1591,7 +1592,8 @@ key_end:
 		    key->byteorder == BYTEORDER_HOST_ENDIAN)
 			mpz_switch_byteorder(key->value, key->len / BITS_PER_BYTE);
 
-		if (key->dtype->basetype != NULL &&
+		if (!(set->flags & NFT_SET_INTERVAL) &&
+		    key->dtype->basetype != NULL &&
 		    key->dtype->basetype->type == TYPE_BITMASK)
 			key = bitmask_expr_to_binops(key);
 	} else if (flags & NFT_SET_ELEM_CATCHALL) {
diff --git a/src/segtree.c b/src/segtree.c
index 5e178fa3c48d..1dc94688aee1 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -411,6 +411,13 @@ void concat_range_aggregate(struct expr *set)
 			assert(r1->etype == EXPR_VALUE && r1->etype == EXPR_VALUE);
 
 			if (!mpz_cmp(r1->value, r2->value)) {
+				if (r2->dtype->basetype != NULL &&
+				    r2->dtype->basetype->type == TYPE_BITMASK) {
+					expr_get(r2);
+					tmp = bitmask_expr_to_binops(r2);
+					list_replace(&r2->list, &tmp->list);
+					expr_free(r2);
+				}
 				free_r1 = 1;
 				goto next;
 			}
@@ -590,6 +597,14 @@ add_interval(struct expr *set, struct expr *low, struct expr *i, bool closed)
 		if (expr_basetype(low)->type == TYPE_STRING)
 			mpz_switch_byteorder(key->value,
 					     key->len / BITS_PER_BYTE);
+
+		if (key->dtype->basetype != NULL &&
+		    key->dtype->basetype->type == TYPE_BITMASK) {
+			if (low->key->etype == EXPR_MAPPING)
+				low->key->left = bitmask_expr_to_binops(low->key->left);
+			else
+				low->key = bitmask_expr_to_binops(low->key);
+		}
 		low->key->flags |= EXPR_F_KERNEL;
 		expr = expr_get(low);
 	} else if (range_is_prefix(range) && !mpz_cmp_ui(p, 0)) {
diff --git a/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.json-nft b/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.json-nft
index 8868f2eb180e..de285354d86a 100644
--- a/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.json-nft
+++ b/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.json-nft
@@ -97,6 +97,65 @@
         ]
       }
     },
+    {
+      "set": {
+        "family": "ip",
+        "name": "s",
+        "table": "x",
+        "type": {
+          "typeof": {
+            "concat": [
+              {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "flags"
+                }
+              },
+              {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "dport"
+                }
+              }
+            ]
+          }
+        },
+        "handle": 0,
+        "flags": [
+          "interval"
+        ],
+        "elem": [
+          {
+            "concat": [
+              {
+                "|": [
+                  "syn",
+                  "ack"
+                ]
+              },
+              80
+            ]
+          },
+          {
+            "concat": [
+              "ack",
+              90
+            ]
+          },
+          {
+            "concat": [
+              "rst",
+              {
+                "range": [
+                  100,
+                  110
+                ]
+              }
+            ]
+          }
+        ]
+      }
+    },
     {
       "rule": {
         "family": "ip",
@@ -204,6 +263,101 @@
           }
         ]
       }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "tcp",
+                      "field": "flags"
+                    }
+                  },
+                  {
+                    "payload": {
+                      "protocol": "tcp",
+                      "field": "dport"
+                    }
+                  }
+                ]
+              },
+              "right": "@s"
+            }
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "concat": [
+                  {
+                    "payload": {
+                      "protocol": "tcp",
+                      "field": "flags"
+                    }
+                  },
+                  {
+                    "payload": {
+                      "protocol": "tcp",
+                      "field": "dport"
+                    }
+                  }
+                ]
+              },
+              "right": {
+                "set": [
+                  {
+                    "concat": [
+                      {
+                        "|": [
+                          "syn",
+                          "ack"
+                        ]
+                      },
+                      80
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "ack",
+                      90
+                    ]
+                  },
+                  {
+                    "concat": [
+                      "rst",
+                      {
+                        "range": [
+                          100,
+                          110
+                        ]
+                      }
+                    ]
+                  }
+                ]
+              }
+            }
+          }
+        ]
+      }
     }
   ]
 }
diff --git a/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.nft b/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.nft
index bc74535b852f..b5d6de32a838 100644
--- a/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.nft
+++ b/tests/shell/testcases/maps/dumps/vmap_concat_range_binary.nft
@@ -7,8 +7,18 @@ table ip x {
 			     rst . 100-110 : drop }
 	}
 
+	set s {
+		typeof tcp flags . tcp dport
+		flags interval
+		elements = { syn | ack . 80,
+			     ack . 90,
+			     rst . 100-110 }
+	}
+
 	chain z {
 		tcp flags . tcp dport vmap @y
 		tcp flags . tcp dport vmap { syn | ack . 80 : accept, ack . 90 : drop, rst . 100-110 : drop }
+		tcp flags . tcp dport @s
+		tcp flags . tcp dport { syn | ack . 80, ack . 90, rst . 100-110 }
 	}
 }
diff --git a/tests/shell/testcases/maps/dumps/vmap_range_binary.json-nft b/tests/shell/testcases/maps/dumps/vmap_range_binary.json-nft
new file mode 100644
index 000000000000..eba4afb013d8
--- /dev/null
+++ b/tests/shell/testcases/maps/dumps/vmap_range_binary.json-nft
@@ -0,0 +1,257 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "ip",
+        "name": "x",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "ip",
+        "table": "x",
+        "name": "z",
+        "handle": 0
+      }
+    },
+    {
+      "map": {
+        "family": "ip",
+        "name": "y",
+        "table": "x",
+        "type": {
+          "typeof": {
+            "payload": {
+              "protocol": "tcp",
+              "field": "flags"
+            }
+          }
+        },
+        "handle": 0,
+        "map": "verdict",
+        "flags": [
+          "interval"
+        ],
+        "elem": [
+          [
+            "rst",
+            {
+              "drop": null
+            }
+          ],
+          [
+            "ack",
+            {
+              "drop": null
+            }
+          ],
+          [
+            {
+              "|": [
+                "syn",
+                "ack"
+              ]
+            },
+            {
+              "accept": null
+            }
+          ],
+          [
+            {
+              "range": [
+                "urg",
+                255
+              ]
+            },
+            {
+              "drop": null
+            }
+          ]
+        ]
+      }
+    },
+    {
+      "set": {
+        "family": "ip",
+        "name": "s",
+        "table": "x",
+        "type": {
+          "typeof": {
+            "payload": {
+              "protocol": "tcp",
+              "field": "flags"
+            }
+          }
+        },
+        "handle": 0,
+        "flags": [
+          "interval"
+        ],
+        "elem": [
+          "rst",
+          "ack",
+          {
+            "|": [
+              "syn",
+              "ack"
+            ]
+          },
+          {
+            "range": [
+              "urg",
+              255
+            ]
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "vmap": {
+              "key": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "flags"
+                }
+              },
+              "data": "@y"
+            }
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "vmap": {
+              "key": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "flags"
+                }
+              },
+              "data": {
+                "set": [
+                  [
+                    "rst",
+                    {
+                      "drop": null
+                    }
+                  ],
+                  [
+                    "ack",
+                    {
+                      "drop": null
+                    }
+                  ],
+                  [
+                    {
+                      "|": [
+                        "syn",
+                        "ack"
+                      ]
+                    },
+                    {
+                      "accept": null
+                    }
+                  ],
+                  [
+                    {
+                      "range": [
+                        "urg",
+                        255
+                      ]
+                    },
+                    {
+                      "drop": null
+                    }
+                  ]
+                ]
+              }
+            }
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "flags"
+                }
+              },
+              "right": "@s"
+            }
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "x",
+        "chain": "z",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "flags"
+                }
+              },
+              "right": {
+                "set": [
+                  "rst",
+                  "ack",
+                  {
+                    "|": [
+                      "syn",
+                      "ack"
+                    ]
+                  },
+                  {
+                    "range": [
+                      "urg",
+                      255
+                    ]
+                  }
+                ]
+              }
+            }
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/tests/shell/testcases/maps/dumps/vmap_range_binary.nft b/tests/shell/testcases/maps/dumps/vmap_range_binary.nft
new file mode 100644
index 000000000000..fbf599c07ff1
--- /dev/null
+++ b/tests/shell/testcases/maps/dumps/vmap_range_binary.nft
@@ -0,0 +1,23 @@
+table ip x {
+	map y {
+		typeof tcp flags : verdict
+		flags interval
+		elements = { rst : drop,
+			     ack : drop,
+			     syn | ack : accept,
+			     0x20-0xff : drop }
+	}
+
+	set s {
+		typeof tcp flags
+		flags interval
+		elements = { rst, ack, syn | ack, 0x20-0xff }
+	}
+
+	chain z {
+		tcp flags vmap @y
+		tcp flags vmap { rst : drop, ack : drop, syn | ack : accept, 0x20-0xff : drop }
+		tcp flags @s
+		tcp flags { rst, ack, syn | ack, 0x20-0xff }
+	}
+}
diff --git a/tests/shell/testcases/maps/vmap_concat_range_binary b/tests/shell/testcases/maps/vmap_concat_range_binary
index ad5ef4a9177e..bd0e5ed8d500 100755
--- a/tests/shell/testcases/maps/vmap_concat_range_binary
+++ b/tests/shell/testcases/maps/vmap_concat_range_binary
@@ -13,9 +13,19 @@ RULESET="table ip x {
 			     rst . 100-110 : drop }
         }
 
+        set s {
+                typeof tcp flags . tcp dport
+                flags interval
+                elements = { syn | ack . 80,
+                             ack . 90,
+                             rst . 100-110 }
+	}
+
         chain z {
                 tcp flags . tcp dport vmap @y
                 tcp flags . tcp dport vmap { syn | ack . 80 : accept, ack . 90 : drop, rst . 100-110 : drop }
+                tcp flags . tcp dport @s
+                tcp flags . tcp dport { syn | ack . 80, ack . 90, rst . 100-110 }
         }
 }"
 
diff --git a/tests/shell/testcases/maps/vmap_range_binary b/tests/shell/testcases/maps/vmap_range_binary
new file mode 100755
index 000000000000..428cec646aca
--- /dev/null
+++ b/tests/shell/testcases/maps/vmap_range_binary
@@ -0,0 +1,35 @@
+#!/bin/bash
+
+set -e
+
+RULESET="table ip x {
+        map y {
+                typeof tcp flags : verdict
+                flags interval
+                elements = { syn | ack : accept,
+                             ack : drop,
+                             rst : drop,
+                             0x20-0xff : drop,
+		}
+        }
+
+        set s {
+                typeof tcp flags
+                flags interval
+                elements = { syn | ack,
+                             ack,
+			     rst,
+			     0x20-0xff,
+		}
+        }
+
+
+        chain z {
+                tcp flags vmap @y
+                tcp flags vmap { syn | ack : accept, ack : drop, rst : drop, 0x20-0xff : drop }
+                tcp flags @s
+                tcp flags { syn | ack, ack, rst, 0x20-0xff }
+        }
+}"
+
+$NFT -f - <<< $RULESET
-- 
2.47.3

Re: [PATCH nft 1/3] segtree: assert on value expressions

[Pablo Neira Ayuso]

Hi,

On Mon, Aug 24, 2026 at 11:29:07PM +0200, Pablo Neira Ayuso wrote:
> Assert value expression before performing math on them instead of
> crashing with unsupported expressions.

JFR: I have pushed out this series.