* [PATCH nft v2 0/3] fix icmpv6 id dependeny handling
@ 2021-06-15 16:01 Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Florian Westphal @ 2021-06-15 16:01 UTC (permalink / raw)
To: netfilter-devel
v2: in patch 1, make sure set has elements and is anonymous.
Pablo reported following bug:
input: icmpv6 id 1
output: icmpv6 type { echo-request, echo-reply } icmpv6 parameter-problem 65536/16
First patch fixes delinearization to handle this correctly.
Second patch fixes a bug related to dependency removal.
Third patch adds test cases for this bug.
src/netlink_delinearize.c | 68 ++++++++++++++++++++++++++++++++++++--
src/payload.c | 61 ++++++++++++++++++++--------------
tests/py/ip/icmp.t | 1
tests/py/ip/icmp.t.json | 28 +++++++++++++++
tests/py/ip/icmp.t.payload.ip | 9 +++++
tests/py/ip6/icmpv6.t | 3 +
tests/py/ip6/icmpv6.t.json | 61 ++++++++++++++++++++++++++++++++++
tests/py/ip6/icmpv6.t.payload.ip6 | 21 +++++++++++
8 files changed, 225 insertions(+), 27 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
2021-06-15 16:01 [PATCH nft v2 0/3] fix icmpv6 id dependeny handling Florian Westphal
@ 2021-06-15 16:01 ` Florian Westphal
2021-06-30 15:13 ` Phil Sutter
2021-06-15 16:01 ` [PATCH nft v2 2/3] payload: do not remove icmp echo dependency Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 3/3] tests: add a icmp-reply only and icmpv6 id test cases Florian Westphal
2 siblings, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2021-06-15 16:01 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Pablo reports following input and output:
in: icmpv6 id 1
out: icmpv6 type { echo-request, echo-reply } icmpv6 parameter-problem 65536/16
Reason is that icmp fields overlap, decoding of the correct name requires
check of the icmpv6 type. This only works for equality tests, for
instance
in: icmpv6 type echo-request icmpv6 id 1
will be listed as "icmpv6 id 1" (which is not correct either, since the
input only matches on echo-request).
with this patch, output of 'icmpv6 id 1' is
icmpv6 type { echo-request, echo-reply } icmpv6 id 1
The second problem, the removal of a single check (request OR reply),
is resolved in the followup patch.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
v2: make sure dependency set candidate a) has elements and b) is
anonymous.
src/netlink_delinearize.c | 68 +++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 3 deletions(-)
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 9a1cf3c4f7d9..2785a9490682 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1819,9 +1819,6 @@ static void payload_match_expand(struct rule_pp_ctx *ctx,
enum proto_bases base = left->payload.base;
bool stacked;
- if (ctx->pdctx.icmp_type)
- ctx->pctx.th_dep.icmp.type = ctx->pdctx.icmp_type;
-
payload_expr_expand(&list, left, &ctx->pctx);
list_for_each_entry(left, &list, list) {
@@ -1868,6 +1865,58 @@ static void payload_match_expand(struct rule_pp_ctx *ctx,
ctx->stmt = NULL;
}
+static void payload_icmp_check(struct rule_pp_ctx *rctx, struct expr *expr, const struct expr *value)
+{
+ const struct proto_hdr_template *tmpl;
+ const struct proto_desc *desc;
+ uint8_t icmp_type;
+ unsigned int i;
+
+ assert(expr->etype == EXPR_PAYLOAD);
+ assert(value->etype == EXPR_VALUE);
+
+ if (expr->payload.base != PROTO_BASE_TRANSPORT_HDR)
+ return;
+
+ /* icmp(v6) type is 8 bit, if value is smaller or larger, this is not
+ * a protocol dependency.
+ */
+ if (expr->len != 8 || value->len != 8 || rctx->pctx.th_dep.icmp.type)
+ return;
+
+ desc = rctx->pctx.protocol[expr->payload.base].desc;
+ if (desc == NULL)
+ return;
+
+ /* not icmp? ignore. */
+ if (desc != &proto_icmp && desc != &proto_icmp6)
+ return;
+
+ assert(desc->base == expr->payload.base);
+
+ icmp_type = mpz_get_uint8(value->value);
+
+ for (i = 1; i < array_size(desc->templates); i++) {
+ tmpl = &desc->templates[i];
+
+ if (tmpl->len == 0)
+ return;
+
+ if (tmpl->offset != expr->payload.offset ||
+ tmpl->len != expr->len)
+ continue;
+
+ /* Matches but doesn't load a protocol key -> ignore. */
+ if (desc->protocol_key != i)
+ return;
+
+ expr->payload.desc = desc;
+ expr->payload.tmpl = tmpl;
+ rctx->pctx.th_dep.icmp.type = icmp_type;
+ return;
+ }
+}
+
static void payload_match_postprocess(struct rule_pp_ctx *ctx,
struct expr *expr,
struct expr *payload)
@@ -1883,6 +1932,19 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
if (expr->right->etype == EXPR_VALUE) {
payload_match_expand(ctx, expr, payload);
break;
+ } else if (expr->right->etype == EXPR_SET_REF) {
+ struct set *set = expr->right->set;
+
+ if (set_is_anonymous(set->flags) &&
+ !list_empty(&set->init->expressions)) {
+ struct expr *elem;
+
+ elem = list_first_entry(&set->init->expressions, struct expr, list);
+
+ if (elem->etype == EXPR_SET_ELEM &&
+ elem->key->etype == EXPR_VALUE)
+ payload_icmp_check(ctx, payload, elem->key);
+ }
}
/* Fall through */
default:
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nft v2 2/3] payload: do not remove icmp echo dependency
2021-06-15 16:01 [PATCH nft v2 0/3] fix icmpv6 id dependeny handling Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
@ 2021-06-15 16:01 ` Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 3/3] tests: add a icmp-reply only and icmpv6 id test cases Florian Westphal
2 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2021-06-15 16:01 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
"icmp type echo-request icmp id 2" and "icmp id 2" are not the same,
the latter gains an implicit dependency on both echo-request and
echo-reply.
Change payload dependency tracking to not store dependency in case
the value type is ICMP(6)_ECHO(REPLY).
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/payload.c | 61 +++++++++++++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 24 deletions(-)
diff --git a/src/payload.c b/src/payload.c
index cfa952248a15..97b60713e800 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -98,12 +98,16 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
desc = proto_find_upper(base, proto);
if (!desc) {
- if (base == &proto_icmp || base == &proto_icmp6) {
+ if (base == &proto_icmp) {
/* proto 0 is ECHOREPLY, just pretend its ECHO.
* Not doing this would need an additional marker
* bit to tell when icmp.type was set.
*/
ctx->th_dep.icmp.type = proto ? proto : ICMP_ECHO;
+ } else if (base == &proto_icmp6) {
+ if (proto == ICMP6_ECHO_REPLY)
+ proto = ICMP6_ECHO_REQUEST;
+ ctx->th_dep.icmp.type = proto;
}
return;
}
@@ -554,33 +558,39 @@ void payload_dependency_reset(struct payload_dep_ctx *ctx)
memset(ctx, 0, sizeof(*ctx));
}
-static uint8_t icmp_get_type(const struct proto_desc *desc, uint8_t value)
+static bool payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx,
+ const struct stmt *stmt)
{
- if (desc == &proto_icmp && value == 0)
- return ICMP_ECHO;
+ struct expr *dep = stmt->expr;
+ const struct proto_desc *desc;
+ const struct expr *right;
+ uint8_t type;
- return value;
-}
+ if (dep->left->etype != EXPR_PAYLOAD)
+ return false;
-static uint8_t icmp_get_dep_type(const struct proto_desc *desc, struct expr *right)
-{
- if (right->etype == EXPR_VALUE && right->len == BITS_PER_BYTE)
- return icmp_get_type(desc, mpz_get_uint8(right->value));
+ right = dep->right;
+ if (right->etype != EXPR_VALUE || right->len != BITS_PER_BYTE)
+ return false;
- return 0;
-}
+ desc = dep->left->payload.desc;
+ if (desc == &proto_icmp) {
+ type = mpz_get_uint8(right->value);
-static void payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx)
-{
- struct expr *dep = ctx->pdep->expr;
- const struct proto_desc *desc;
+ if (type == ICMP_ECHOREPLY)
+ type = ICMP_ECHO;
- if (dep->left->etype != EXPR_PAYLOAD)
- return;
+ ctx->icmp_type = type;
- desc = dep->left->payload.desc;
- if (desc == &proto_icmp || desc == &proto_icmp6)
- ctx->icmp_type = icmp_get_dep_type(dep->left->payload.desc, dep->right);
+ return type == ICMP_ECHO;
+ } else if (desc == &proto_icmp6) {
+ type = mpz_get_uint8(right->value);
+
+ ctx->icmp_type = type;
+ return type == ICMP6_ECHO_REQUEST || type == ICMP6_ECHO_REPLY;
+ }
+
+ return false;
}
/**
@@ -593,10 +603,13 @@ static void payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx)
void payload_dependency_store(struct payload_dep_ctx *ctx,
struct stmt *stmt, enum proto_bases base)
{
- ctx->pbase = base + 1;
- ctx->pdep = stmt;
+ bool ignore_dep = payload_dependency_store_icmp_type(ctx, stmt);
+
+ if (ignore_dep)
+ return;
- payload_dependency_store_icmp_type(ctx);
+ ctx->pdep = stmt;
+ ctx->pbase = base + 1;
}
/**
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH nft v2 3/3] tests: add a icmp-reply only and icmpv6 id test cases
2021-06-15 16:01 [PATCH nft v2 0/3] fix icmpv6 id dependeny handling Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 2/3] payload: do not remove icmp echo dependency Florian Westphal
@ 2021-06-15 16:01 ` Florian Westphal
2 siblings, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2021-06-15 16:01 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Check that nft doesn't remove the dependency in these cases:
icmp type echo-reply icmp id 1
("icmp id" matches both echo request and reply).
Add icmpv6 test cases. These fail without the previous patches:
add rule ip6 test-ip6 input icmpv6 id 1:
'icmpv6 id 1' mismatches
'icmpv6 type { echo-request, echo-reply} icmpv6 parameter-problem 65536/16'
add rule ip6 test-ip6 input icmpv6 type echo-reply icmpv6 id 65534':
'icmpv6 type echo-reply icmpv6 id 65534' mismatches
'icmpv6 type echo-reply @th,32,16 65534'
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tests/py/ip/icmp.t | 1 +
tests/py/ip/icmp.t.json | 28 ++++++++++++++
tests/py/ip/icmp.t.payload.ip | 9 +++++
tests/py/ip6/icmpv6.t | 3 ++
tests/py/ip6/icmpv6.t.json | 61 +++++++++++++++++++++++++++++++
tests/py/ip6/icmpv6.t.payload.ip6 | 21 +++++++++++
6 files changed, 123 insertions(+)
diff --git a/tests/py/ip/icmp.t b/tests/py/ip/icmp.t
index fd89af0dff20..7ddf8b38a538 100644
--- a/tests/py/ip/icmp.t
+++ b/tests/py/ip/icmp.t
@@ -53,6 +53,7 @@ icmp sequence { 33, 55, 67, 88};ok;icmp type { echo-request, echo-reply} icmp se
icmp sequence != { 33, 55, 67, 88};ok;icmp type { echo-request, echo-reply} icmp sequence != { 33, 55, 67, 88}
icmp id 1 icmp sequence 2;ok;icmp type { echo-reply, echo-request} icmp id 1 icmp sequence 2
icmp type { echo-reply, echo-request} icmp id 1 icmp sequence 2;ok
+icmp type echo-reply icmp id 1;ok
icmp mtu 33;ok
icmp mtu 22-33;ok
diff --git a/tests/py/ip/icmp.t.json b/tests/py/ip/icmp.t.json
index 576335cc63d2..4f0525094cf0 100644
--- a/tests/py/ip/icmp.t.json
+++ b/tests/py/ip/icmp.t.json
@@ -1123,6 +1123,34 @@
}
]
+# icmp type echo-reply icmp id 1
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "type",
+ "protocol": "icmp"
+ }
+ },
+ "op": "==",
+ "right": "echo-reply"
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "id",
+ "protocol": "icmp"
+ }
+ },
+ "op": "==",
+ "right": 1
+ }
+ }
+]
+
# icmp mtu 33
[
{
diff --git a/tests/py/ip/icmp.t.payload.ip b/tests/py/ip/icmp.t.payload.ip
index 024739c0c3cc..3bc6de3cf717 100644
--- a/tests/py/ip/icmp.t.payload.ip
+++ b/tests/py/ip/icmp.t.payload.ip
@@ -413,6 +413,15 @@ ip
[ payload load 4b @ transport header + 4 => reg 1 ]
[ cmp eq reg 1 0x02000100 ]
+# icmp type echo-reply icmp id 1
+ip
+ [ meta load l4proto => reg 1 ]
+ [ cmp eq reg 1 0x00000001 ]
+ [ payload load 1b @ transport header + 0 => reg 1 ]
+ [ cmp eq reg 1 0x00000000 ]
+ [ payload load 2b @ transport header + 4 => reg 1 ]
+ [ cmp eq reg 1 0x00000100 ]
+
# icmp mtu 33
ip test-ip4 input
[ meta load l4proto => reg 1 ]
diff --git a/tests/py/ip6/icmpv6.t b/tests/py/ip6/icmpv6.t
index c8d4cffcd9d7..4de6ee2377dd 100644
--- a/tests/py/ip6/icmpv6.t
+++ b/tests/py/ip6/icmpv6.t
@@ -67,6 +67,9 @@ icmpv6 id != 33-45;ok;icmpv6 type { echo-request, echo-reply} icmpv6 id != 33-45
icmpv6 id {33, 55, 67, 88};ok;icmpv6 type { echo-request, echo-reply} icmpv6 id { 33, 55, 67, 88}
icmpv6 id != {33, 55, 67, 88};ok;icmpv6 type { echo-request, echo-reply} icmpv6 id != { 33, 55, 67, 88}
+icmpv6 id 1;ok;icmpv6 type { echo-request, echo-reply} icmpv6 id 1
+icmpv6 type echo-reply icmpv6 id 65534;ok
+
icmpv6 sequence 2;ok;icmpv6 type { echo-request, echo-reply} icmpv6 sequence 2
icmpv6 sequence {3, 4, 5, 6, 7} accept;ok;icmpv6 type { echo-request, echo-reply} icmpv6 sequence { 3, 4, 5, 6, 7} accept
diff --git a/tests/py/ip6/icmpv6.t.json b/tests/py/ip6/icmpv6.t.json
index 30d2ad988185..2251be82a39e 100644
--- a/tests/py/ip6/icmpv6.t.json
+++ b/tests/py/ip6/icmpv6.t.json
@@ -856,6 +856,67 @@
}
]
+# icmpv6 id 1
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "type",
+ "protocol": "icmpv6"
+ }
+ },
+ "op": "==",
+ "right": {
+ "set": [
+ "echo-request",
+ "echo-reply"
+ ]
+ }
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "id",
+ "protocol": "icmpv6"
+ }
+ },
+ "op": "==",
+ "right": 1
+ }
+ }
+]
+
+# icmpv6 type echo-reply icmpv6 id 65534
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "type",
+ "protocol": "icmpv6"
+ }
+ },
+ "op": "==",
+ "right": "echo-reply"
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "id",
+ "protocol": "icmpv6"
+ }
+ },
+ "op": "==",
+ "right": 65534
+ }
+ }
+]
+
# icmpv6 sequence 2
[
{
diff --git a/tests/py/ip6/icmpv6.t.payload.ip6 b/tests/py/ip6/icmpv6.t.payload.ip6
index 76df184cd0d0..0e96be2d0788 100644
--- a/tests/py/ip6/icmpv6.t.payload.ip6
+++ b/tests/py/ip6/icmpv6.t.payload.ip6
@@ -407,6 +407,27 @@ ip6 test-ip6 input
[ payload load 2b @ transport header + 4 => reg 1 ]
[ lookup reg 1 set __set%d 0x1 ]
+# icmpv6 id 1
+__set%d test-ip6 3 size 2
+__set%d test-ip6 0
+ element 00000080 : 0 [end] element 00000081 : 0 [end]
+ip6
+ [ meta load l4proto => reg 1 ]
+ [ cmp eq reg 1 0x0000003a ]
+ [ payload load 1b @ transport header + 0 => reg 1 ]
+ [ lookup reg 1 set __set%d ]
+ [ payload load 2b @ transport header + 4 => reg 1 ]
+ [ cmp eq reg 1 0x00000100 ]
+
+# icmpv6 type echo-reply icmpv6 id 65534
+ip6
+ [ meta load l4proto => reg 1 ]
+ [ cmp eq reg 1 0x0000003a ]
+ [ payload load 1b @ transport header + 0 => reg 1 ]
+ [ cmp eq reg 1 0x00000081 ]
+ [ payload load 2b @ transport header + 4 => reg 1 ]
+ [ cmp eq reg 1 0x0000feff ]
+
# icmpv6 sequence 2
__set%d test-ip6 3
__set%d test-ip6 0
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
@ 2021-06-30 15:13 ` Phil Sutter
2021-06-30 15:34 ` Florian Westphal
2021-06-30 15:58 ` Florian Westphal
0 siblings, 2 replies; 8+ messages in thread
From: Phil Sutter @ 2021-06-30 15:13 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, Pablo Neira Ayuso, Eric Garver
Hi,
On Tue, Jun 15, 2021 at 06:01:49PM +0200, Florian Westphal wrote:
> Pablo reports following input and output:
> in: icmpv6 id 1
> out: icmpv6 type { echo-request, echo-reply } icmpv6 parameter-problem 65536/16
>
> Reason is that icmp fields overlap, decoding of the correct name requires
> check of the icmpv6 type. This only works for equality tests, for
> instance
>
> in: icmpv6 type echo-request icmpv6 id 1
> will be listed as "icmpv6 id 1" (which is not correct either, since the
> input only matches on echo-request).
>
> with this patch, output of 'icmpv6 id 1' is
> icmpv6 type { echo-request, echo-reply } icmpv6 id 1
>
> The second problem, the removal of a single check (request OR reply),
> is resolved in the followup patch.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Eric reported a testcase in which this patch seems to cause a segfault
(bisected). The test is as simple as:
| nft -f - <<EOF
| add table inet firewalld_check_rule_index
| add chain inet firewalld_check_rule_index foobar { type filter hook input priority 0 ; }
| add rule inet firewalld_check_rule_index foobar tcp dport 1234 accept
| add rule inet firewalld_check_rule_index foobar accept
| insert rule inet firewalld_check_rule_index foobar index 1 udp dport 4321 accept
| EOF
But a ruleset is in place at this time. Also, I can't reproduce it on my
own machine but only on Eric's VM for testing.
[...]
> static void payload_match_postprocess(struct rule_pp_ctx *ctx,
> struct expr *expr,
> struct expr *payload)
> @@ -1883,6 +1932,19 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
> if (expr->right->etype == EXPR_VALUE) {
> payload_match_expand(ctx, expr, payload);
> break;
> + } else if (expr->right->etype == EXPR_SET_REF) {
> + struct set *set = expr->right->set;
> +
> + if (set_is_anonymous(set->flags) &&
> + !list_empty(&set->init->expressions)) {
According to GDB, set->init is NULL here.
I am not familiar with recent changes in cache code, maybe there's the
actual culprit: Debug printf in cache_init_objects() states flags
variable is 0x4000005f, i.e. NFT_CACHE_SETELEM_BIT is not set.
I am not sure if caching is incomplete and we need that bit or if the
above code should expect sets with missing elements and therefore check
'set->init != NULL' before accessing expressions field.
Cheers, Phil
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
2021-06-30 15:13 ` Phil Sutter
@ 2021-06-30 15:34 ` Florian Westphal
2021-06-30 15:58 ` Florian Westphal
1 sibling, 0 replies; 8+ messages in thread
From: Florian Westphal @ 2021-06-30 15:34 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel,
Pablo Neira Ayuso, Eric Garver
Phil Sutter <phil@nwl.cc> wrote:
> > will be listed as "icmpv6 id 1" (which is not correct either, since the
> > input only matches on echo-request).
> >
> > with this patch, output of 'icmpv6 id 1' is
> > icmpv6 type { echo-request, echo-reply } icmpv6 id 1
> >
> > The second problem, the removal of a single check (request OR reply),
> > is resolved in the followup patch.
> >
> > Signed-off-by: Florian Westphal <fw@strlen.de>
>
> Eric reported a testcase in which this patch seems to cause a segfault
> (bisected). The test is as simple as:
>
> | nft -f - <<EOF
> | add table inet firewalld_check_rule_index
> | add chain inet firewalld_check_rule_index foobar { type filter hook input priority 0 ; }
> | add rule inet firewalld_check_rule_index foobar tcp dport 1234 accept
> | add rule inet firewalld_check_rule_index foobar accept
> | insert rule inet firewalld_check_rule_index foobar index 1 udp dport 4321 accept
> | EOF
>
> But a ruleset is in place at this time. Also, I can't reproduce it on my
> own machine but only on Eric's VM for testing.
You need to add a ruleset with
icmp id 42
then it will crash. adding 'list ruleset' before EOF avoids it,
because cache gets populated.
> I am not familiar with recent changes in cache code, maybe there's the
> actual culprit: Debug printf in cache_init_objects() states flags
> variable is 0x4000005f, i.e. NFT_CACHE_SETELEM_BIT is not set.
>
> I am not sure if caching is incomplete and we need that bit or if the
> above code should expect sets with missing elements and therefore check
> 'set->init != NULL' before accessing expressions field.
I think correct fix is to check set->init, we don't need to do
postprocessing if its not going to be printed.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
2021-06-30 15:13 ` Phil Sutter
2021-06-30 15:34 ` Florian Westphal
@ 2021-06-30 15:58 ` Florian Westphal
2021-06-30 17:12 ` Phil Sutter
1 sibling, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2021-06-30 15:58 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel,
Pablo Neira Ayuso, Eric Garver
Phil Sutter <phil@nwl.cc> wrote:
> Eric reported a testcase in which this patch seems to cause a segfault
> (bisected). The test is as simple as:
I've pushed fix and a test case.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
2021-06-30 15:58 ` Florian Westphal
@ 2021-06-30 17:12 ` Phil Sutter
0 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2021-06-30 17:12 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, Pablo Neira Ayuso, Eric Garver
Hey,
On Wed, Jun 30, 2021 at 05:58:26PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > Eric reported a testcase in which this patch seems to cause a segfault
> > (bisected). The test is as simple as:
>
> I've pushed fix and a test case.
Awesome, thanks for the quick patch and test case!
Cheers, Phil
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-30 17:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 16:01 [PATCH nft v2 0/3] fix icmpv6 id dependeny handling Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
2021-06-30 15:13 ` Phil Sutter
2021-06-30 15:34 ` Florian Westphal
2021-06-30 15:58 ` Florian Westphal
2021-06-30 17:12 ` Phil Sutter
2021-06-15 16:01 ` [PATCH nft v2 2/3] payload: do not remove icmp echo dependency Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 3/3] tests: add a icmp-reply only and icmpv6 id test cases Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).