* [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value
@ 2025-01-30 17:47 Florian Westphal
2025-01-30 17:47 ` [PATCH nft 2/3] src: add and use payload_expr_trim_force Florian Westphal
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Florian Westphal @ 2025-01-30 17:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal, Sunny73Cr
Given following input:
table ip t {
chain c {
@ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
}
}
nft will produce following output:
chain c {
@ih,48,16 set @ih,48,16 & 0x3f @ih,80,16 set @ih,80,16 & 0x3f0 @ih,160,32 set @ih,160,32 & 0x3fffff
}
The input side is correct, the generated expressions sent to kernel are:
1 [ payload load 2b @ inner header + 6 => reg 1 ]
2 [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
3 [ payload write reg 1 => 2b @ inner header + 6 .. ]
4 [ payload load 2b @ inner header + 10 => reg 1 ]
5 [ bitwise reg 1 = ( reg 1 & 0x00000ffc ) ^ 0x00000000 ]
6 [ payload write reg 1 => 2b @ inner header + 10 .. ]
7 [ payload load 4b @ inner header + 20 => reg 1 ]
8 [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
9 [ payload write reg 1 => 4b @ inner header + 20 .. ]
@ih,58,6 set 0 <- Zero 6 bits, starting with bit 58
Changes to inner header mandate a checksum update, which only works for
even byte counts (except for last byte in the payload).
Thus, we load 2b at offet 6. (16bits, offset 48).
Because we want to zero 6 bits, we need a mask that retains 10 bits and
clears 6: b1111111111000000 (first 8 bit retains 48-57, last 6 bit clear
58-63). The '0xc0ff' is not correct, but thats because debug output comes
from libnftnl which prints values in host byte order, the value will be
interpreted as big endian on kernel side, so this will do the right thing.
Next, same problem:
@ih,86,6 set 0 <- Zero 6 bits, starting with bit 86.
nft needs to round down to even-sized byte offset, 10, then retain first
6 bits (80 + 6 == 86), then clear 6 bits (86-91), then keep 4 more as-is
(92-95).
So mask is 0xfc0f (in big endian) would be correct (b1111110000001111).
Last expression, @ih,170,22 set 0, asks to clear 22 bits starting with bit
170, nft correctly rounds this down to a 32 bit read at offset 160.
Required mask keeps first 10 bits, then clears 22
(b11111111110000000000000000000000). Required mask would be 0xffc00000,
which corresponds to the wrong-endian-printed value in line 8 above.
Now that we convinced ourselves that the input side is correct, fix up
netlink delinearize to undo the mask alterations if we can't find a
template to print a human-readable payload expression.
With this patch, we get this output:
@ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set @ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000
... which isn't ideal. We should fixup the payload expression to display
the same output as the input, i.e. adjust payload->len and offset as per
mask and discard the mask instead.
This will be done in a followup patch.
Fixes: 50ca788ca4d0 ("netlink: decode payload statment")
Reported-by: Sunny73Cr <Sunny73Cr@protonmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/netlink_delinearize.c | 57 ++++++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index db8b6bbe13e8..046e7a472b8d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -3226,41 +3226,54 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
break;
}
case EXPR_PAYLOAD: /* II? */
- value = expr->right;
- if (value->etype != EXPR_VALUE)
+ payload = expr->left;
+ mask = expr->right;
+
+ if (mask->etype != EXPR_VALUE)
+ return;
+
+ if (!payload_expr_cmp(stmt->payload.expr, payload))
return;
switch (expr->op) {
- case OP_AND: /* IIa */
- payload = expr->left;
+ case OP_AND: { /* IIa */
+ mpz_t tmp;
+
+ mpz_init(tmp);
+ mpz_set(tmp, mask->value);
+
mpz_init_bitmask(bitmask, payload->len);
- mpz_xor(bitmask, bitmask, value->value);
- mpz_set(value->value, bitmask);
+ mpz_xor(bitmask, bitmask, mask->value);
+ mpz_set(mask->value, bitmask);
mpz_clear(bitmask);
- break;
- case OP_OR: /* IIb */
- break;
- default: /* No idea */
- return;
- }
- stmt_payload_binop_pp(ctx, expr);
- if (!payload_is_known(expr->left))
- return;
+ stmt_payload_binop_pp(ctx, expr);
+ if (!payload_is_known(expr->left)) {
+ mpz_set(mask->value, tmp);
+ mpz_clear(tmp);
+ return;
+ }
- expr_free(stmt->payload.expr);
+ mpz_clear(tmp);
- switch (expr->op) {
- case OP_AND:
- /* Mask was used to match payload, i.e.
- * user asked to set zero value.
+ /* Mask was used to match payload, i.e. user asked to
+ * clear the payload expression.
+ * The "mask" value becomes new stmt->payload.value
+ * so set this to 0.
*/
- mpz_set_ui(value->value, 0);
+ mpz_set_ui(mask->value, 0);
break;
- default:
+ }
+ case OP_OR: /* IIb */
+ stmt_payload_binop_pp(ctx, expr);
+ if (!payload_is_known(expr->left))
+ return;
break;
+ default: /* No idea what to do */
+ return;
}
+ expr_free(stmt->payload.expr);
stmt->payload.expr = expr_get(expr->left);
stmt->payload.val = expr_get(expr->right);
expr_free(expr);
--
2.45.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH nft 2/3] src: add and use payload_expr_trim_force
2025-01-30 17:47 [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Florian Westphal
@ 2025-01-30 17:47 ` Florian Westphal
2025-02-07 9:54 ` Pablo Neira Ayuso
2025-01-30 17:47 ` [PATCH nft 3/3] tests: py: extend raw payload match tests Florian Westphal
2025-02-07 9:54 ` [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Pablo Neira Ayuso
2 siblings, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2025-01-30 17:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Previous commit fixed erroneous handling of raw expressions when RHS sets
a zero value.
Input: @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
Output:@ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set \
@ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000
After this patch, this will instead display:
@ih,58,6 set 0x0 @ih,86,6 set 0x0 @ih,170,22 set 0x0
payload_expr_trim_force() only works when the payload has no known
protocol (template) attached, i.e. will be printed as raw payload syntax.
It performs sanity checks on @mask and then adjusts the payload expression
length and offset according to the mask.
Also add this check in __binop_postprocess() so we can also discard masks
when matching, e.g.
'@ih,7,5 2' becomes '@ih,7,5 0x2', not '@ih,0,16 & 0xffc0 == 0x20'.
binop_postprocess now returns if it performed an action or not; if this
returns true then arguments might have been freed so callers must no longer
refer to any of the expressions attached to the binop.
Next patch adds test cases for this.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/payload.h | 2 ++
src/netlink_delinearize.c | 31 +++++++++++++++++-----
src/payload.c | 54 +++++++++++++++++++++++++++++++++++++++
3 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/include/payload.h b/include/payload.h
index 08e45f7f79e2..6685dad6f9f7 100644
--- a/include/payload.h
+++ b/include/payload.h
@@ -62,6 +62,8 @@ extern struct expr *payload_expr_join(const struct expr *e1,
bool payload_expr_trim(struct expr *expr, struct expr *mask,
const struct proto_ctx *ctx, unsigned int *shift);
+bool payload_expr_trim_force(struct expr *expr, struct expr *mask,
+ unsigned int *shift);
extern void payload_expr_expand(struct list_head *list, struct expr *expr,
const struct proto_ctx *ctx);
extern void payload_expr_complete(struct expr *expr,
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 046e7a472b8d..86c8602860f6 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2451,7 +2451,7 @@ static void binop_adjust(const struct expr *binop, struct expr *right,
}
}
-static void __binop_postprocess(struct rule_pp_ctx *ctx,
+static bool __binop_postprocess(struct rule_pp_ctx *ctx,
struct expr *expr,
struct expr *left,
struct expr *mask,
@@ -2501,17 +2501,27 @@ static void __binop_postprocess(struct rule_pp_ctx *ctx,
expr_set_type(right, left->dtype, left->byteorder);
expr_free(binop);
+ return true;
+ } else if (left->etype == EXPR_PAYLOAD &&
+ expr->right->etype == EXPR_VALUE &&
+ payload_expr_trim_force(left, mask, &shift)) {
+ mpz_rshift_ui(expr->right->value, shift);
+ *expr_binop = expr_get(left);
+ expr_free(binop);
+ return true;
}
+
+ return false;
}
-static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr,
+static bool 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);
+ return __binop_postprocess(ctx, expr, left, mask, expr_binop);
}
static void map_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
@@ -3178,6 +3188,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
switch (expr->left->etype) {
case EXPR_BINOP: {/* I? */
+ unsigned int shift = 0;
mpz_t tmp;
if (expr->op != OP_OR)
@@ -3211,13 +3222,18 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
mpz_set(mask->value, bitmask);
mpz_clear(bitmask);
- binop_postprocess(ctx, expr, &expr->left);
- if (!payload_is_known(payload)) {
+ if (!binop_postprocess(ctx, expr, &expr->left) &&
+ !payload_is_known(payload) &&
+ !payload_expr_trim_force(payload,
+ mask, &shift)) {
mpz_set(mask->value, tmp);
mpz_clear(tmp);
return;
}
+ if (shift)
+ mpz_rshift_ui(value->value, shift);
+
mpz_clear(tmp);
expr_free(stmt->payload.expr);
stmt->payload.expr = expr_get(payload);
@@ -3237,6 +3253,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
switch (expr->op) {
case OP_AND: { /* IIa */
+ unsigned int shift_unused;
mpz_t tmp;
mpz_init(tmp);
@@ -3248,7 +3265,8 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
mpz_clear(bitmask);
stmt_payload_binop_pp(ctx, expr);
- if (!payload_is_known(expr->left)) {
+ if (!payload_is_known(expr->left) &&
+ !payload_expr_trim_force(expr->left, mask, &shift_unused)) {
mpz_set(mask->value, tmp);
mpz_clear(tmp);
return;
@@ -3260,6 +3278,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
* clear the payload expression.
* The "mask" value becomes new stmt->payload.value
* so set this to 0.
+ * Also the reason why &shift_unused is ignored.
*/
mpz_set_ui(mask->value, 0);
break;
diff --git a/src/payload.c b/src/payload.c
index 44aa834cc07b..f8b192b5f2fa 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -1046,6 +1046,7 @@ static unsigned int mask_length(const struct expr *mask)
* @expr: the payload expression
* @mask: mask to use when searching templates
* @ctx: protocol context
+ * @shift: shift adjustment to fix up RHS value
*
* Walk the template list and determine if a match can be found without
* using the provided mask.
@@ -1106,6 +1107,59 @@ bool payload_expr_trim(struct expr *expr, struct expr *mask,
return false;
}
+/**
+ * payload_expr_trim_force - adjust payload len/offset according to mask
+ *
+ * @expr: the payload expression
+ * @mask: mask to use when searching templates
+ * @shift: shift adjustment to fix up RHS value
+ *
+ * Force-trim an unknown payload expression according to mask.
+ *
+ * This is only useful for unkown payload expressions that need
+ * to be printed in raw syntax (@base,offset,length). The kernel
+ * can only deal with byte-divisible offsets/length, e.g. @th,16,8.
+ * In such case we might be able to get rid of the mask.
+ * @base,offset,length & MASK OPERATOR VALUE then becomes
+ * @base,offset,length VALUE, where at least one of offset increases
+ * and length decreases.
+ *
+ * This function also returns the shift for the right hand
+ * constant side of the expression.
+ *
+ * @return: true if @expr was adjusted and mask can be discarded.
+ */
+bool payload_expr_trim_force(struct expr *expr, struct expr *mask, unsigned int *shift)
+{
+ unsigned int payload_offset = expr->payload.offset;
+ unsigned int mask_len = mask_length(mask);
+ unsigned int off, real_len;
+
+ if (payload_is_known(expr) || expr->len <= mask_len)
+ return false;
+
+ /* This provides the payload offset to use.
+ * mask->len is the total length of the mask, e.g. 16.
+ * mask_len holds the last bit number that will be zeroed,
+ */
+ off = round_up(mask->len, BITS_PER_BYTE) - mask_len;
+ payload_offset += off;
+
+ /* kernel only allows offsets <= 255 */
+ if (round_up(payload_offset, BITS_PER_BYTE) > 255)
+ return false;
+
+ real_len = mpz_popcount(mask->value);
+ if (real_len > expr->len)
+ return false;
+
+ expr->payload.offset = payload_offset;
+ expr->len = real_len;
+
+ *shift = mask_to_offset(mask);
+ return true;
+}
+
/**
* payload_expr_expand - expand raw merged adjacent payload expressions into its
* original components
--
2.45.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH nft 3/3] tests: py: extend raw payload match tests
2025-01-30 17:47 [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Florian Westphal
2025-01-30 17:47 ` [PATCH nft 2/3] src: add and use payload_expr_trim_force Florian Westphal
@ 2025-01-30 17:47 ` Florian Westphal
2025-02-07 9:54 ` Pablo Neira Ayuso
2025-02-07 9:54 ` [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Pablo Neira Ayuso
2 siblings, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2025-01-30 17:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Add more test cases to exercise binop elimination for raw
payload matches.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/py/any/rawpayload.t | 7 ++
tests/py/any/rawpayload.t.json | 157 ++++++++++++++++++++++++++++++
tests/py/any/rawpayload.t.payload | 53 ++++++++++
3 files changed, 217 insertions(+)
diff --git a/tests/py/any/rawpayload.t b/tests/py/any/rawpayload.t
index 5bc9d35f7465..745b4a615e6c 100644
--- a/tests/py/any/rawpayload.t
+++ b/tests/py/any/rawpayload.t
@@ -15,6 +15,7 @@ meta l4proto tcp @th,16,16 { 22, 23, 80};ok;tcp dport { 22, 23, 80}
@ll,0,0 2;fail
@ll,0,1;fail
+@ll,1,0 1;fail
@ll,0,1 1;ok;@ll,0,8 & 0x80 == 0x80
@ll,0,8 & 0x80 == 0x80;ok
@ll,0,128 0xfedcba987654321001234567890abcde;ok
@@ -22,3 +23,9 @@ meta l4proto tcp @th,16,16 { 22, 23, 80};ok;tcp dport { 22, 23, 80}
meta l4proto 91 @th,400,16 0x0 accept;ok
@ih,32,32 0x14000000;ok
+@ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0;ok;@ih,58,6 set 0x0 @ih,86,6 set 0x0 @ih,170,22 set 0x0
+@ih,58,6 set 0x1 @ih,86,6 set 0x2 @ih,170,22 set 0x3;ok
+@ih,58,6 0x0 @ih,86,6 0x0 @ih,170,22 0x0;ok
+@ih,1,1 0x2;fail
+@ih,1,2 0x2;ok
+@ih,35,3 0x2;ok
diff --git a/tests/py/any/rawpayload.t.json b/tests/py/any/rawpayload.t.json
index 4cae4d493da3..4a06c5987a7b 100644
--- a/tests/py/any/rawpayload.t.json
+++ b/tests/py/any/rawpayload.t.json
@@ -204,3 +204,160 @@
}
]
+# @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
+[
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 58
+ }
+ },
+ "value": 0
+ }
+ },
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 86
+ }
+ },
+ "value": 0
+ }
+ },
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 22,
+ "offset": 170
+ }
+ },
+ "value": 0
+ }
+ }
+]
+
+# @ih,58,6 set 0x1 @ih,86,6 set 0x2 @ih,170,22 set 0x3
+[
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 58
+ }
+ },
+ "value": 1
+ }
+ },
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 86
+ }
+ },
+ "value": 2
+ }
+ },
+ {
+ "mangle": {
+ "key": {
+ "payload": {
+ "base": "ih",
+ "len": 22,
+ "offset": 170
+ }
+ },
+ "value": 3
+ }
+ }
+]
+
+# @ih,58,6 0x0 @ih,86,6 0x0 @ih,170,22 0x0
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 58
+ }
+ },
+ "op": "==",
+ "right": 0
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "base": "ih",
+ "len": 6,
+ "offset": 86
+ }
+ },
+ "op": "==",
+ "right": 0
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "base": "ih",
+ "len": 22,
+ "offset": 170
+ }
+ },
+ "op": "==",
+ "right": 0
+ }
+ }
+]
+
+# @ih,1,2 0x2
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "base": "ih",
+ "len": 2,
+ "offset": 1
+ }
+ },
+ "op": "==",
+ "right": 2
+ }
+ }
+]
+
+# @ih,35,3 0x2
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "base": "ih",
+ "len": 3,
+ "offset": 35
+ }
+ },
+ "op": "==",
+ "right": 2
+ }
+ }
+]
+
diff --git a/tests/py/any/rawpayload.t.payload b/tests/py/any/rawpayload.t.payload
index fe2377e65a77..8984eef6a481 100644
--- a/tests/py/any/rawpayload.t.payload
+++ b/tests/py/any/rawpayload.t.payload
@@ -61,3 +61,56 @@ inet test-inet input
[ payload load 4b @ inner header + 4 => reg 1 ]
[ cmp eq reg 1 0x00000014 ]
+# @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
+inet test-inet input
+ [ payload load 2b @ inner header + 6 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
+ [ payload write reg 1 => 2b @ inner header + 6 csum_type 0 csum_off 0 csum_flags 0x1 ]
+ [ payload load 2b @ inner header + 10 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x00000ffc ) ^ 0x00000000 ]
+ [ payload write reg 1 => 2b @ inner header + 10 csum_type 0 csum_off 0 csum_flags 0x1 ]
+ [ payload load 4b @ inner header + 20 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
+ [ payload write reg 1 => 4b @ inner header + 20 csum_type 0 csum_off 0 csum_flags 0x1 ]
+
+# @ih,58,6 set 0x1 @ih,86,6 set 0x2 @ih,170,22 set 0x3
+inet test-inet input
+ [ payload load 2b @ inner header + 6 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000100 ]
+ [ payload write reg 1 => 2b @ inner header + 6 csum_type 0 csum_off 0 csum_flags 0x1 ]
+ [ payload load 2b @ inner header + 10 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x00000ffc ) ^ 0x00002000 ]
+ [ payload write reg 1 => 2b @ inner header + 10 csum_type 0 csum_off 0 csum_flags 0x1 ]
+ [ payload load 4b @ inner header + 20 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x03000000 ]
+ [ payload write reg 1 => 4b @ inner header + 20 csum_type 0 csum_off 0 csum_flags 0x1 ]
+
+# @ih,58,6 0x0 @ih,86,6 0x0 @ih,170,22 0x0
+inet test-inet input
+ [ payload load 1b @ inner header + 7 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000003f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000000 ]
+ [ payload load 2b @ inner header + 10 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000f003 ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000000 ]
+ [ payload load 3b @ inner header + 21 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x00ffff3f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000000 ]
+
+# @ih,1,2 0x2
+inet test-inet input
+ [ payload load 1b @ inner header + 0 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x00000060 ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000040 ]
+
+# @ih,2,1 0x1
+inet test-inet input
+ [ payload load 1b @ inner header + 0 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x00000020 ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000020 ]
+
+# @ih,35,3 0x2
+inet test-inet input
+ [ payload load 1b @ inner header + 4 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000001c ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000008 ]
--
2.45.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value
2025-01-30 17:47 [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Florian Westphal
2025-01-30 17:47 ` [PATCH nft 2/3] src: add and use payload_expr_trim_force Florian Westphal
2025-01-30 17:47 ` [PATCH nft 3/3] tests: py: extend raw payload match tests Florian Westphal
@ 2025-02-07 9:54 ` Pablo Neira Ayuso
2 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-02-07 9:54 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, Sunny73Cr
On Thu, Jan 30, 2025 at 06:47:12PM +0100, Florian Westphal wrote:
> Given following input:
> table ip t {
> chain c {
> @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
> }
> }
>
> nft will produce following output:
> chain c {
> @ih,48,16 set @ih,48,16 & 0x3f @ih,80,16 set @ih,80,16 & 0x3f0 @ih,160,32 set @ih,160,32 & 0x3fffff
> }
>
> The input side is correct, the generated expressions sent to kernel are:
>
> 1 [ payload load 2b @ inner header + 6 => reg 1 ]
> 2 [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
> 3 [ payload write reg 1 => 2b @ inner header + 6 .. ]
> 4 [ payload load 2b @ inner header + 10 => reg 1 ]
> 5 [ bitwise reg 1 = ( reg 1 & 0x00000ffc ) ^ 0x00000000 ]
> 6 [ payload write reg 1 => 2b @ inner header + 10 .. ]
> 7 [ payload load 4b @ inner header + 20 => reg 1 ]
> 8 [ bitwise reg 1 = ( reg 1 & 0x0000c0ff ) ^ 0x00000000 ]
> 9 [ payload write reg 1 => 4b @ inner header + 20 .. ]
>
> @ih,58,6 set 0 <- Zero 6 bits, starting with bit 58
>
> Changes to inner header mandate a checksum update, which only works for
> even byte counts (except for last byte in the payload).
>
> Thus, we load 2b at offet 6. (16bits, offset 48).
>
> Because we want to zero 6 bits, we need a mask that retains 10 bits and
> clears 6: b1111111111000000 (first 8 bit retains 48-57, last 6 bit clear
> 58-63). The '0xc0ff' is not correct, but thats because debug output comes
> from libnftnl which prints values in host byte order, the value will be
> interpreted as big endian on kernel side, so this will do the right thing.
>
> Next, same problem:
>
> @ih,86,6 set 0 <- Zero 6 bits, starting with bit 86.
>
> nft needs to round down to even-sized byte offset, 10, then retain first
> 6 bits (80 + 6 == 86), then clear 6 bits (86-91), then keep 4 more as-is
> (92-95).
>
> So mask is 0xfc0f (in big endian) would be correct (b1111110000001111).
>
> Last expression, @ih,170,22 set 0, asks to clear 22 bits starting with bit
> 170, nft correctly rounds this down to a 32 bit read at offset 160.
>
> Required mask keeps first 10 bits, then clears 22
> (b11111111110000000000000000000000). Required mask would be 0xffc00000,
> which corresponds to the wrong-endian-printed value in line 8 above.
>
> Now that we convinced ourselves that the input side is correct, fix up
> netlink delinearize to undo the mask alterations if we can't find a
> template to print a human-readable payload expression.
>
> With this patch, we get this output:
>
> @ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set @ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000
>
> ... which isn't ideal. We should fixup the payload expression to display
> the same output as the input, i.e. adjust payload->len and offset as per
> mask and discard the mask instead.
>
> This will be done in a followup patch.
>
> Fixes: 50ca788ca4d0 ("netlink: decode payload statment")
> Reported-by: Sunny73Cr <Sunny73Cr@protonmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft 2/3] src: add and use payload_expr_trim_force
2025-01-30 17:47 ` [PATCH nft 2/3] src: add and use payload_expr_trim_force Florian Westphal
@ 2025-02-07 9:54 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-02-07 9:54 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Thu, Jan 30, 2025 at 06:47:13PM +0100, Florian Westphal wrote:
> Previous commit fixed erroneous handling of raw expressions when RHS sets
> a zero value.
>
> Input: @ih,58,6 set 0 @ih,86,6 set 0 @ih,170,22 set 0
> Output:@ih,48,16 set @ih,48,16 & 0xffc0 @ih,80,16 set \
> @ih,80,16 & 0xfc0f @ih,160,32 set @ih,160,32 & 0xffc00000
>
> After this patch, this will instead display:
>
> @ih,58,6 set 0x0 @ih,86,6 set 0x0 @ih,170,22 set 0x0
>
> payload_expr_trim_force() only works when the payload has no known
> protocol (template) attached, i.e. will be printed as raw payload syntax.
>
> It performs sanity checks on @mask and then adjusts the payload expression
> length and offset according to the mask.
>
> Also add this check in __binop_postprocess() so we can also discard masks
> when matching, e.g.
>
> '@ih,7,5 2' becomes '@ih,7,5 0x2', not '@ih,0,16 & 0xffc0 == 0x20'.
>
> binop_postprocess now returns if it performed an action or not; if this
> returns true then arguments might have been freed so callers must no longer
> refer to any of the expressions attached to the binop.
>
> Next patch adds test cases for this.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH nft 3/3] tests: py: extend raw payload match tests
2025-01-30 17:47 ` [PATCH nft 3/3] tests: py: extend raw payload match tests Florian Westphal
@ 2025-02-07 9:54 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2025-02-07 9:54 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Thu, Jan 30, 2025 at 06:47:14PM +0100, Florian Westphal wrote:
> Add more test cases to exercise binop elimination for raw
> payload matches.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-07 9:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-30 17:47 [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Florian Westphal
2025-01-30 17:47 ` [PATCH nft 2/3] src: add and use payload_expr_trim_force Florian Westphal
2025-02-07 9:54 ` Pablo Neira Ayuso
2025-01-30 17:47 ` [PATCH nft 3/3] tests: py: extend raw payload match tests Florian Westphal
2025-02-07 9:54 ` Pablo Neira Ayuso
2025-02-07 9:54 ` [PATCH nft 1/3] netlink_delinarize: fix bogus munging of mask value Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.