From e20eac092725c982387181f3df557877b02eb345 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Fri, 15 Apr 2016 15:09:39 +0200 Subject: payload: only merge if adjacent and combined size fits into a register add rule ip6 filter input ip6 saddr ::1/128 ip6 daddr ::1/128 fails, we ask to compare a 32byte immediate which is not supported: [ payload load 32b @ network header + 8 => reg 1 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 0x00000000 0x00000000 0x00000000 0x02000000 ] We would need to use two cmps in this case, i.e.: [ payload load 32b @ network header + 8 => reg 1 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 ] [ cmp eq reg 2 0x00000000 0x00000000 0x00000000 0x02000000 ] Seems however that this requires a bit more changes to how nft handles register allocations, we'd also need to undo the constant merge. Lets disable merging for now so that we generate [ payload load 16b @ network header + 8 => reg 1 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 ] [ payload load 16b @ network header + 24 => reg 1 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x02000000 ] ... if merge would bring us over the 128 bit register size. Closes: http://bugzilla.netfilter.org/show_bug.cgi?id=1032 Signed-off-by: Florian Westphal Acked-by: Arturo Borrero Gonzalez Acked-by: Pablo Neira Ayuso --- include/payload.h | 2 +- src/payload.c | 27 ++++++++++++++++++++------- src/rule.c | 2 +- tests/py/ip6/ip6.t | 1 + tests/py/ip6/ip6.t.payload.inet | 9 +++++++++ tests/py/ip6/ip6.t.payload.ip6 | 7 +++++++ 6 files changed, 39 insertions(+), 9 deletions(-) diff --git a/include/payload.h b/include/payload.h index a19e6906..fae3c673 100644 --- a/include/payload.h +++ b/include/payload.h @@ -17,7 +17,7 @@ extern int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr, extern int exthdr_gen_dependency(struct eval_ctx *ctx, const struct expr *expr, struct stmt **res); -extern bool payload_is_adjacent(const struct expr *e1, const struct expr *e2); +extern bool payload_can_merge(const struct expr *e1, const struct expr *e2); extern struct expr *payload_expr_join(const struct expr *e1, const struct expr *e2); diff --git a/src/payload.c b/src/payload.c index 7e380617..f6c0a975 100644 --- a/src/payload.c +++ b/src/payload.c @@ -483,23 +483,36 @@ raw: list_add_tail(&new->list, list); } +static bool payload_is_adjacent(const struct expr *e1, const struct expr *e2) +{ + if (e1->payload.base == e2->payload.base && + e1->payload.offset + e1->len == e2->payload.offset) + return true; + return false; +} + /** - * payload_is_adjacent - return whether two payload expressions refer to - * adjacent header locations + * payload_can_merge - return whether two payload expressions can be merged * * @e1: first payload expression * @e2: second payload expression */ -bool payload_is_adjacent(const struct expr *e1, const struct expr *e2) +bool payload_can_merge(const struct expr *e1, const struct expr *e2) { + unsigned int total; + + if (!payload_is_adjacent(e1, e2)) + return false; + if (e1->payload.offset % BITS_PER_BYTE || e1->len % BITS_PER_BYTE || e2->payload.offset % BITS_PER_BYTE || e2->len % BITS_PER_BYTE) return false; - if (e1->payload.base == e2->payload.base && - e1->payload.offset + e1->len == e2->payload.offset) - return true; - return false; + total = e1->len + e2->len; + if (total < e1->len || total > (NFT_REG_SIZE * BITS_PER_BYTE)) + return false; + + return true; } /** diff --git a/src/rule.c b/src/rule.c index b7f4a07f..2fe67454 100644 --- a/src/rule.c +++ b/src/rule.c @@ -1292,7 +1292,7 @@ static void payload_do_merge(struct stmt *sa[], unsigned int n) stmt = sa[i]; this = stmt->expr; - if (!payload_is_adjacent(last->left, this->left) || + if (!payload_can_merge(last->left, this->left) || last->op != this->op) { last = this; j = i; diff --git a/tests/py/ip6/ip6.t b/tests/py/ip6/ip6.t index 96366721..2a1eee78 100644 --- a/tests/py/ip6/ip6.t +++ b/tests/py/ip6/ip6.t @@ -139,6 +139,7 @@ ip6 saddr 1234::;ok # v61 ({v610}|{v611}) # v60 (::) ip6 saddr ::/64;ok +ip6 saddr ::1 ip6 daddr ::2;ok - ip6 daddr != {::1234:1234:1234:1234:1234:1234:1234, 1234:1234::1234:1234:1234:1234:1234 };ok ip6 daddr != ::1234:1234:1234:1234:1234:1234:1234-1234:1234::1234:1234:1234:1234:1234;ok;ip6 daddr != 0:1234:1234:1234:1234:1234:1234:1234-1234:1234:0:1234:1234:1234:1234:1234 diff --git a/tests/py/ip6/ip6.t.payload.inet b/tests/py/ip6/ip6.t.payload.inet index 4bafb267..4d2ea352 100644 --- a/tests/py/ip6/ip6.t.payload.inet +++ b/tests/py/ip6/ip6.t.payload.inet @@ -449,6 +449,15 @@ inet test-inet input [ bitwise reg 1 = (reg=1 & 0xffffffff 0xffffffff 0x00000000 0x00000000 ) ^ 0x00000000 0x00000000 0x00000000 0x00000000 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x00000000 ] +# ip6 saddr ::1 ip6 daddr ::2 +inet test-inet input + [ meta load nfproto => reg 1 ] + [ cmp eq reg 1 0x0000000a ] + [ payload load 16b @ network header + 8 => reg 1 ] + [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 ] + [ payload load 16b @ network header + 24 => reg 1 ] + [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x02000000 ] + # ip6 daddr != ::1234:1234:1234:1234:1234:1234:1234-1234:1234::1234:1234:1234:1234:1234 inet test-inet input [ meta load nfproto => reg 1 ] diff --git a/tests/py/ip6/ip6.t.payload.ip6 b/tests/py/ip6/ip6.t.payload.ip6 index c6e856cc..db59bfbe 100644 --- a/tests/py/ip6/ip6.t.payload.ip6 +++ b/tests/py/ip6/ip6.t.payload.ip6 @@ -329,6 +329,13 @@ ip6 test-ip6 input [ bitwise reg 1 = (reg=1 & 0xffffffff 0xffffffff 0x00000000 0x00000000 ) ^ 0x00000000 0x00000000 0x00000000 0x00000000 ] [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x00000000 ] +# ip6 saddr ::1 ip6 daddr ::2 +ip6 test-ip6 input + [ payload load 16b @ network header + 8 => reg 1 ] + [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x01000000 ] + [ payload load 16b @ network header + 24 => reg 1 ] + [ cmp eq reg 1 0x00000000 0x00000000 0x00000000 0x02000000 ] + # ip6 daddr != ::1234:1234:1234:1234:1234:1234:1234-1234:1234::1234:1234:1234:1234:1234 ip6 test-ip6 input [ payload load 16b @ network header + 24 => reg 1 ] -- cgit v1.2.3