!78 backport upstream patches

From: @ultra_planet 
Reviewed-by: @sunsuwan 
Signed-off-by: @sunsuwan
This commit is contained in:
openeuler-ci-bot 2024-04-19 09:34:11 +00:00 committed by Gitee
commit 69857e1ee0
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
39 changed files with 2748 additions and 1 deletions

View File

@ -0,0 +1,44 @@
From 1f867d0d07122f54f76e20af3c636ce66102b683 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 24 Oct 2023 11:57:07 +0200
Subject: [PATCH] datatype: don't return a const string from
cgroupv2_get_path()
The caller is supposed to free the allocated string. Return a non-const
string to make that clearer.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/datatype.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/datatype.c b/src/datatype.c
index 64e4647a..63627358 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1465,10 +1465,10 @@ const struct datatype policy_type = {
#define SYSFS_CGROUPSV2_PATH "/sys/fs/cgroup"
-static const char *cgroupv2_get_path(const char *path, uint64_t id)
+static char *cgroupv2_get_path(const char *path, uint64_t id)
{
- const char *cgroup_path = NULL;
char dent_name[PATH_MAX + 1];
+ char *cgroup_path = NULL;
struct dirent *dent;
struct stat st;
DIR *d;
@@ -1506,7 +1506,7 @@ static void cgroupv2_type_print(const struct expr *expr,
struct output_ctx *octx)
{
uint64_t id = mpz_get_uint64(expr->value);
- const char *cgroup_path;
+ char *cgroup_path;
cgroup_path = cgroupv2_get_path(SYSFS_CGROUPSV2_PATH, id);
if (cgroup_path)
--
2.33.0

View File

@ -0,0 +1,485 @@
From 8519ab031d8022999603a69ee9f18e8cfb06645d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Sep 2023 09:30:54 +0200
Subject: [PATCH] datatype: fix leak and cleanup reference counting for struct
datatype
Test `./tests/shell/run-tests.sh -V tests/shell/testcases/maps/nat_addr_port`
fails:
==118== 195 (112 direct, 83 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==118== at 0x484682C: calloc (vg_replace_malloc.c:1554)
==118== by 0x48A39DD: xmalloc (utils.c:37)
==118== by 0x48A39DD: xzalloc (utils.c:76)
==118== by 0x487BDFD: datatype_alloc (datatype.c:1205)
==118== by 0x487BDFD: concat_type_alloc (datatype.c:1288)
==118== by 0x488229D: stmt_evaluate_nat_map (evaluate.c:3786)
==118== by 0x488229D: stmt_evaluate_nat (evaluate.c:3892)
==118== by 0x488229D: stmt_evaluate (evaluate.c:4450)
==118== by 0x488328E: rule_evaluate (evaluate.c:4956)
==118== by 0x48ADC71: nft_evaluate (libnftables.c:552)
==118== by 0x48AEC29: nft_run_cmd_from_buffer (libnftables.c:595)
==118== by 0x402983: main (main.c:534)
I think the reference handling for datatype is wrong. It was introduced
by commit 01a13882bb59 ('src: add reference counter for dynamic
datatypes').
We don't notice it most of the time, because instances are statically
allocated, where datatype_get()/datatype_free() is a NOP.
Fix and rework.
- Commit 01a13882bb59 comments "The reference counter of any newly
allocated datatype is set to zero". That seems not workable.
Previously, functions like datatype_clone() would have returned the
refcnt set to zero. Some callers would then then set the refcnt to one, but
some wouldn't (set_datatype_alloc()). Calling datatype_free() with a
refcnt of zero will overflow to UINT_MAX and leak:
if (--dtype->refcnt > 0)
return;
While there could be schemes with such asymmetric counting that juggle the
appropriate number of datatype_get() and datatype_free() calls, this is
confusing and error prone. The common pattern is that every
alloc/clone/get/ref is paired with exactly one unref/free.
Let datatype_clone() return references with refcnt set 1 and in
general be always clear about where we transfer ownership (take a
reference) and where we need to release it.
- set_datatype_alloc() needs to consistently return ownership to the
reference. Previously, some code paths would and others wouldn't.
- Replace
datatype_set(key, set_datatype_alloc(dtype, key->byteorder))
with a __datatype_set() with takes ownership.
Fixes: 01a13882bb59 ('src: add reference counter for dynamic datatypes')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/datatype.h | 1 +
include/expression.h | 4 +++
src/datatype.c | 23 ++++++++++----
src/evaluate.c | 66 ++++++++++++++++++++++++---------------
src/expression.c | 2 +-
src/netlink.c | 31 +++++++++---------
src/netlink_delinearize.c | 2 +-
src/payload.c | 3 +-
8 files changed, 82 insertions(+), 50 deletions(-)
diff --git a/include/datatype.h b/include/datatype.h
index 6146eda1..52a2e943 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -176,6 +176,7 @@ extern const struct datatype *datatype_lookup(enum datatypes type);
extern const struct datatype *datatype_lookup_byname(const char *name);
extern struct datatype *datatype_get(const struct datatype *dtype);
extern void datatype_set(struct expr *expr, const struct datatype *dtype);
+extern void __datatype_set(struct expr *expr, const struct datatype *dtype);
extern void datatype_free(const struct datatype *dtype);
struct datatype *dtype_clone(const struct datatype *orig_dtype);
diff --git a/include/expression.h b/include/expression.h
index 733dd3cf..469f41ec 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -120,7 +120,11 @@ enum symbol_types {
* @maxval: expected maximum value
*/
struct expr_ctx {
+ /* expr_ctx does not own the reference to dtype. The caller must ensure
+ * the valid lifetime.
+ */
const struct datatype *dtype;
+
enum byteorder byteorder;
unsigned int len;
unsigned int maxval;
diff --git a/src/datatype.c b/src/datatype.c
index 678a16b1..70c84846 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1204,6 +1204,7 @@ static struct datatype *datatype_alloc(void)
dtype = xzalloc(sizeof(*dtype));
dtype->flags = DTYPE_F_ALLOC;
+ dtype->refcnt = 1;
return dtype;
}
@@ -1221,12 +1222,19 @@ struct datatype *datatype_get(const struct datatype *ptr)
return dtype;
}
+void __datatype_set(struct expr *expr, const struct datatype *dtype)
+{
+ const struct datatype *dtype_free;
+
+ dtype_free = expr->dtype;
+ expr->dtype = dtype;
+ datatype_free(dtype_free);
+}
+
void datatype_set(struct expr *expr, const struct datatype *dtype)
{
- if (dtype == expr->dtype)
- return;
- datatype_free(expr->dtype);
- expr->dtype = datatype_get(dtype);
+ if (dtype != expr->dtype)
+ __datatype_set(expr, datatype_get(dtype));
}
struct datatype *dtype_clone(const struct datatype *orig_dtype)
@@ -1238,7 +1246,7 @@ struct datatype *datatype_clone(const struct datatype *orig_dtype)
dtype->name = xstrdup(orig_dtype->name);
dtype->desc = xstrdup(orig_dtype->desc);
dtype->flags = DTYPE_F_ALLOC | orig_dtype->flags;
- dtype->refcnt = 0;
+ dtype->refcnt = 1;
return dtype;
}
@@ -1251,6 +1259,9 @@ void datatype_free(const struct datatype *ptr)
return;
if (!(dtype->flags & DTYPE_F_ALLOC))
return;
+
+ assert(dtype->refcnt != 0);
+
if (--dtype->refcnt > 0)
return;
@@ -1303,7 +1314,7 @@ const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype,
/* Restrict dynamic datatype allocation to generic integer datatype. */
if (orig_dtype != &integer_type)
- return orig_dtype;
+ return datatype_get(orig_dtype);
dtype = dtype_clone(orig_dtype);
dtype->byteorder = byteorder;
diff --git a/src/evaluate.c b/src/evaluate.c
index b0c6919f..1b7e0b37 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -82,7 +82,7 @@ static void key_fix_dtype_byteorder(struct expr *key)
if (dtype->byteorder == key->byteorder)
return;
- datatype_set(key, set_datatype_alloc(dtype, key->byteorder));
+ __datatype_set(key, set_datatype_alloc(dtype, key->byteorder));
}
static int set_evaluate(struct eval_ctx *ctx, struct set *set);
@@ -1521,8 +1521,7 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
clone = dtype_clone(i->dtype);
clone->size = i->len;
clone->byteorder = i->byteorder;
- clone->refcnt = 1;
- i->dtype = clone;
+ __datatype_set(i, clone);
}
if (dtype == NULL && i->dtype->size == 0)
@@ -1550,7 +1549,7 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
}
(*expr)->flags |= flags;
- datatype_set(*expr, concat_type_alloc(ntype));
+ __datatype_set(*expr, concat_type_alloc(ntype));
(*expr)->len = size;
if (off > 0)
@@ -1888,7 +1887,6 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
{
struct expr *map = *expr, *mappings;
struct expr_ctx ectx = ctx->ectx;
- const struct datatype *dtype;
struct expr *key, *data;
if (map->map->etype == EXPR_CT &&
@@ -1930,12 +1928,16 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
ctx->ectx.len, NULL);
}
- dtype = set_datatype_alloc(ectx.dtype, ectx.byteorder);
- if (dtype->type == TYPE_VERDICT)
+ if (ectx.dtype->type == TYPE_VERDICT) {
data = verdict_expr_alloc(&netlink_location, 0, NULL);
- else
+ } else {
+ const struct datatype *dtype;
+
+ dtype = set_datatype_alloc(ectx.dtype, ectx.byteorder);
data = constant_expr_alloc(&netlink_location, dtype,
dtype->byteorder, ectx.len, NULL);
+ datatype_free(dtype);
+ }
mappings = implicit_set_declaration(ctx, "__map%d",
key, data,
@@ -3765,8 +3767,10 @@ static int stmt_evaluate_nat_map(struct eval_ctx *ctx, struct stmt *stmt)
{
struct proto_ctx *pctx = eval_proto_ctx(ctx);
struct expr *one, *two, *data, *tmp;
- const struct datatype *dtype;
- int addr_type, err;
+ const struct datatype *dtype = NULL;
+ const struct datatype *dtype2;
+ int addr_type;
+ int err;
if (stmt->nat.family == NFPROTO_INET)
expr_family_infer(pctx, stmt->nat.addr, &stmt->nat.family);
@@ -3786,18 +3790,23 @@ static int stmt_evaluate_nat_map(struct eval_ctx *ctx, struct stmt *stmt)
dtype = concat_type_alloc((addr_type << TYPE_BITS) | TYPE_INET_SERVICE);
expr_set_context(&ctx->ectx, dtype, dtype->size);
- if (expr_evaluate(ctx, &stmt->nat.addr))
- return -1;
+ if (expr_evaluate(ctx, &stmt->nat.addr)) {
+ err = -1;
+ goto out;
+ }
if (pctx->protocol[PROTO_BASE_TRANSPORT_HDR].desc == NULL &&
!nat_evaluate_addr_has_th_expr(stmt->nat.addr)) {
- return stmt_binary_error(ctx, stmt->nat.addr, stmt,
+ err = stmt_binary_error(ctx, stmt->nat.addr, stmt,
"transport protocol mapping is only "
"valid after transport protocol match");
+ goto out;
}
- if (stmt->nat.addr->etype != EXPR_MAP)
- return 0;
+ if (stmt->nat.addr->etype != EXPR_MAP) {
+ err = 0;
+ goto out;
+ }
data = stmt->nat.addr->mappings->set->data;
if (data->flags & EXPR_F_INTERVAL)
@@ -3805,36 +3814,42 @@ static int stmt_evaluate_nat_map(struct eval_ctx *ctx, struct stmt *stmt)
datatype_set(data, dtype);
- if (expr_ops(data)->type != EXPR_CONCAT)
- return __stmt_evaluate_arg(ctx, stmt, dtype, dtype->size,
+ if (expr_ops(data)->type != EXPR_CONCAT) {
+ err = __stmt_evaluate_arg(ctx, stmt, dtype, dtype->size,
BYTEORDER_BIG_ENDIAN,
&stmt->nat.addr);
+ goto out;
+ }
one = list_first_entry(&data->expressions, struct expr, list);
two = list_entry(one->list.next, struct expr, list);
- if (one == two || !list_is_last(&two->list, &data->expressions))
- return __stmt_evaluate_arg(ctx, stmt, dtype, dtype->size,
+ if (one == two || !list_is_last(&two->list, &data->expressions)) {
+ err = __stmt_evaluate_arg(ctx, stmt, dtype, dtype->size,
BYTEORDER_BIG_ENDIAN,
&stmt->nat.addr);
+ goto out;
+ }
- dtype = get_addr_dtype(stmt->nat.family);
+ dtype2 = get_addr_dtype(stmt->nat.family);
tmp = one;
- err = __stmt_evaluate_arg(ctx, stmt, dtype, dtype->size,
+ err = __stmt_evaluate_arg(ctx, stmt, dtype2, dtype2->size,
BYTEORDER_BIG_ENDIAN,
&tmp);
if (err < 0)
- return err;
+ goto out;
if (tmp != one)
BUG("Internal error: Unexpected alteration of l3 expression");
tmp = two;
err = nat_evaluate_transport(ctx, stmt, &tmp);
if (err < 0)
- return err;
+ goto out;
if (tmp != two)
BUG("Internal error: Unexpected alteration of l4 expression");
+out:
+ datatype_free(dtype);
return err;
}
@@ -4549,8 +4564,7 @@ static int set_expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
dtype = dtype_clone(i->dtype);
dtype->size = i->len;
dtype->byteorder = i->byteorder;
- dtype->refcnt = 1;
- i->dtype = dtype;
+ __datatype_set(i, dtype);
}
if (i->dtype->size == 0 && i->len == 0)
@@ -4573,7 +4587,7 @@ static int set_expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
}
(*expr)->flags |= flags;
- datatype_set(*expr, concat_type_alloc(ntype));
+ __datatype_set(*expr, concat_type_alloc(ntype));
(*expr)->len = size;
expr_set_context(&ctx->ectx, (*expr)->dtype, (*expr)->len);
diff --git a/src/expression.c b/src/expression.c
index cb222a2b..87d5a9fc 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1013,7 +1013,7 @@ static struct expr *concat_expr_parse_udata(const struct nftnl_udata *attr)
if (!dtype)
goto err_free;
- concat_expr->dtype = datatype_get(dtype);
+ __datatype_set(concat_expr, dtype);
concat_expr->len = len;
return concat_expr;
diff --git a/src/netlink.c b/src/netlink.c
index 59cde9a4..4d3c1cf1 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -798,6 +798,10 @@ enum nft_data_types dtype_map_to_kernel(const struct datatype *dtype)
static const struct datatype *dtype_map_from_kernel(enum nft_data_types type)
{
+ /* The function always returns ownership of a reference. But for
+ * &verdict_Type and datatype_lookup(), those are static instances,
+ * we can omit the datatype_get() call.
+ */
switch (type) {
case NFT_DATA_VERDICT:
return &verdict_type;
@@ -933,12 +937,14 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
const struct nftnl_udata *ud[NFTNL_UDATA_SET_MAX + 1] = {};
enum byteorder keybyteorder = BYTEORDER_INVALID;
enum byteorder databyteorder = BYTEORDER_INVALID;
- const struct datatype *keytype, *datatype = NULL;
struct expr *typeof_expr_key, *typeof_expr_data;
struct setelem_parse_ctx set_parse_ctx;
+ const struct datatype *datatype = NULL;
+ const struct datatype *keytype = NULL;
+ const struct datatype *dtype2 = NULL;
+ const struct datatype *dtype = NULL;
const char *udata, *comment = NULL;
uint32_t flags, key, objtype = 0;
- const struct datatype *dtype;
uint32_t data_interval = 0;
bool automerge = false;
struct set *set;
@@ -990,8 +996,8 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
netlink_io_error(ctx, NULL,
"Unknown data type in set key %u",
data);
- datatype_free(keytype);
- return NULL;
+ set = NULL;
+ goto out;
}
}
@@ -1029,19 +1035,18 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
if (datatype) {
uint32_t dlen;
- dtype = set_datatype_alloc(datatype, databyteorder);
+ dtype2 = set_datatype_alloc(datatype, databyteorder);
klen = nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE;
dlen = data_interval ? klen / 2 : klen;
if (set_udata_key_valid(typeof_expr_data, dlen)) {
typeof_expr_data->len = klen;
- datatype_free(datatype_get(dtype));
set->data = typeof_expr_data;
} else {
expr_free(typeof_expr_data);
set->data = constant_expr_alloc(&netlink_location,
- dtype,
+ dtype2,
databyteorder, klen,
NULL);
@@ -1052,16 +1057,12 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
if (data_interval)
set->data->flags |= EXPR_F_INTERVAL;
-
- if (dtype != datatype)
- datatype_free(datatype);
}
dtype = set_datatype_alloc(keytype, keybyteorder);
klen = nftnl_set_get_u32(nls, NFTNL_SET_KEY_LEN) * BITS_PER_BYTE;
if (set_udata_key_valid(typeof_expr_key, klen)) {
- datatype_free(datatype_get(dtype));
set->key = typeof_expr_key;
set->key_typeof_valid = true;
} else {
@@ -1071,9 +1072,6 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
NULL);
}
- if (dtype != keytype)
- datatype_free(keytype);
-
set->flags = nftnl_set_get_u32(nls, NFTNL_SET_FLAGS);
set->handle.handle.id = nftnl_set_get_u64(nls, NFTNL_SET_HANDLE);
@@ -1101,6 +1099,11 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
}
}
+out:
+ datatype_free(datatype);
+ datatype_free(keytype);
+ datatype_free(dtype2);
+ datatype_free(dtype);
return set;
}
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 19c3f0bd..41cb37a3 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2768,7 +2768,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
}
ctx->flags &= ~RULE_PP_IN_CONCATENATION;
list_splice(&tmp, &expr->expressions);
- datatype_set(expr, concat_type_alloc(ntype));
+ __datatype_set(expr, concat_type_alloc(ntype));
break;
}
case EXPR_UNARY:
diff --git a/src/payload.c b/src/payload.c
index dcd87485..a02942b3 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -253,8 +253,7 @@ static struct expr *payload_expr_parse_udata(const struct nftnl_udata *attr)
dtype = dtype_clone(&xinteger_type);
dtype->size = len;
dtype->byteorder = BYTEORDER_BIG_ENDIAN;
- dtype->refcnt = 1;
- expr->dtype = dtype;
+ __datatype_set(expr, dtype);
}
if (ud[NFTNL_UDATA_SET_KEY_PAYLOAD_INNER_DESC]) {
--
2.33.0

View File

@ -0,0 +1,59 @@
From 2d9e23a9e9c3c729784a3add41639cbd3f72d504 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 19 Sep 2023 18:15:17 +0200
Subject: [PATCH] datatype: initialize TYPE_CT_EVENTBIT slot in datatype array
Matching on ct event makes no sense since this is mostly used as
statement to globally filter out ctnetlink events, but do not crash
if it is used from concatenations.
Add the missing slot in the datatype array so this does not crash.
Fixes: 2595b9ad6840 ("ct: add conntrack event mask support")
Reported-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/ct.h | 1 +
src/ct.c | 2 +-
src/datatype.c | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/ct.h b/include/ct.h
index aa0504c5..0a705fd0 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -40,5 +40,6 @@ extern const struct datatype ct_dir_type;
extern const struct datatype ct_state_type;
extern const struct datatype ct_status_type;
extern const struct datatype ct_label_type;
+extern const struct datatype ct_event_type;
#endif /* NFTABLES_CT_H */
diff --git a/src/ct.c b/src/ct.c
index d7dec255..b2635543 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -132,7 +132,7 @@ static const struct symbol_table ct_events_tbl = {
},
};
-static const struct datatype ct_event_type = {
+const struct datatype ct_event_type = {
.type = TYPE_CT_EVENTBIT,
.name = "ct_event",
.desc = "conntrack event bits",
diff --git a/src/datatype.c b/src/datatype.c
index ee0e9701..14d5a0e6 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -75,6 +75,7 @@ static const struct datatype *datatypes[TYPE_MAX + 1] = {
[TYPE_ECN] = &ecn_type,
[TYPE_FIB_ADDR] = &fib_addr_type,
[TYPE_BOOLEAN] = &boolean_type,
+ [TYPE_CT_EVENTBIT] = &ct_event_type,
[TYPE_IFNAME] = &ifname_type,
[TYPE_IGMP_TYPE] = &igmp_type_type,
[TYPE_TIME_DATE] = &date_type,
--
2.33.0

View File

@ -0,0 +1,71 @@
From 1b235f9962a059a599d9a9ecce477ed71e328e89 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 19 Sep 2023 18:09:31 +0200
Subject: [PATCH] datatype: initialize TYPE_CT_LABEL slot in datatype array
Otherwise, ct label with concatenations such as:
table ip x {
chain y {
ct label . ct mark { 0x1 . 0x1 }
}
}
crashes:
../include/datatype.h:196:11: runtime error: member access within null pointer of type 'const struct datatype'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==640948==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc970d3199b bp 0x7fffd1f20560 sp 0x7fffd1f20540 T0)
==640948==The signal is caused by a READ memory access.
==640948==Hint: address points to the zero page.
sudo #0 0x7fc970d3199b in datatype_equal ../include/datatype.h:196
Fixes: 2fcce8b0677b ("ct: connlabel matching support")
Reported-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/ct.h | 1 +
src/ct.c | 2 +-
src/datatype.c | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/ct.h b/include/ct.h
index efb2d418..aa0504c5 100644
--- a/include/ct.h
+++ b/include/ct.h
@@ -39,5 +39,6 @@ extern const char *ct_label2str(const struct symbol_table *tbl,
extern const struct datatype ct_dir_type;
extern const struct datatype ct_state_type;
extern const struct datatype ct_status_type;
+extern const struct datatype ct_label_type;
#endif /* NFTABLES_CT_H */
diff --git a/src/ct.c b/src/ct.c
index 6760b085..d7dec255 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -217,7 +217,7 @@ static struct error_record *ct_label_type_parse(struct parse_ctx *ctx,
return NULL;
}
-static const struct datatype ct_label_type = {
+const struct datatype ct_label_type = {
.type = TYPE_CT_LABEL,
.name = "ct_label",
.desc = "conntrack label",
diff --git a/src/datatype.c b/src/datatype.c
index 70c84846..ee0e9701 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -65,6 +65,7 @@ static const struct datatype *datatypes[TYPE_MAX + 1] = {
[TYPE_CT_DIR] = &ct_dir_type,
[TYPE_CT_STATUS] = &ct_status_type,
[TYPE_ICMP6_TYPE] = &icmp6_type_type,
+ [TYPE_CT_LABEL] = &ct_label_type,
[TYPE_PKTTYPE] = &pkttype_type,
[TYPE_ICMP_CODE] = &icmp_code_type,
[TYPE_ICMPV6_CODE] = &icmpv6_code_type,
--
2.33.0

View File

@ -0,0 +1,53 @@
From 01167c393a12c74c6f9a3015b75702964ff5bcda Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 28 Aug 2023 22:47:05 +0200
Subject: [PATCH] evaluate: do not remove anonymous set with protocol flags and
single element
Set lookups with flags search for an exact match, however:
tcp flags { syn }
gets transformed into:
tcp flags syn
which is matching on the syn flag only (non-exact match).
This optimization is safe for ct state though, because only one bit is
ever set on in the ct state bitmask.
Since protocol flags allow for combining flags, skip this optimization
to retain exact match semantics.
Another possible solution is to turn OP_IMPLICIT into OP_EQ for exact
flag match to re-introduce this optimization and deal with this corner
case.
Fixes: fee6bda06403 ("evaluate: remove anon sets with exactly one element")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index c13be824..b5326d7d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1817,7 +1817,12 @@ static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
set->set_flags |= NFT_SET_CONCAT;
} else if (set->size == 1) {
i = list_first_entry(&set->expressions, struct expr, list);
- if (i->etype == EXPR_SET_ELEM && list_empty(&i->stmt_list)) {
+ if (i->etype == EXPR_SET_ELEM &&
+ (!i->dtype->basetype ||
+ i->dtype->basetype->type != TYPE_BITMASK ||
+ i->dtype->type == TYPE_CT_STATE) &&
+ list_empty(&i->stmt_list)) {
+
switch (i->key->etype) {
case EXPR_PREFIX:
case EXPR_RANGE:
--
2.33.0

View File

@ -0,0 +1,50 @@
From 588470e00539404fd793fe22718067721f5754be Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Wed, 20 Dec 2023 11:06:04 +0100
Subject: [PATCH] evaluate: don't crash if object map does not refer to a value
Before:
BUG: Value export of 512 bytes would overflownft: src/netlink.c:474: netlink_gen_prefix: Assertion `0' failed.
After:
66: Error: Object mapping data should be a value, not prefix
synproxy name ip saddr map { 192.168.1.0/24 : "v*" }
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 5 +++++
tests/shell/testcases/bogons/nft-f/objmap_to_prefix_assert | 6 ++++++
2 files changed, 11 insertions(+)
create mode 100644 tests/shell/testcases/bogons/nft-f/objmap_to_prefix_assert
diff --git a/src/evaluate.c b/src/evaluate.c
index 5ddbde42..26f0110f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2140,6 +2140,11 @@ static int expr_evaluate_mapping(struct eval_ctx *ctx, struct expr **expr)
return expr_error(ctx->msgs, mapping->right,
"Value must be a singleton");
+ if (set_is_objmap(set->flags) && mapping->right->etype != EXPR_VALUE)
+ return expr_error(ctx->msgs, mapping->right,
+ "Object mapping data should be a value, not %s",
+ expr_name(mapping->right));
+
mapping->flags |= EXPR_F_CONSTANT;
return 0;
}
diff --git a/tests/shell/testcases/bogons/nft-f/objmap_to_prefix_assert b/tests/shell/testcases/bogons/nft-f/objmap_to_prefix_assert
new file mode 100644
index 00000000..d880a377
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/objmap_to_prefix_assert
@@ -0,0 +1,6 @@
+table t {
+ chain y {
+ type filter hook input priority filter; policy accept;
+ synproxy name ip saddr map { 192.168.1.0/24 : "x*" }
+ }
+}
--
2.33.0

View File

@ -0,0 +1,43 @@
From 666018e71ebb5df376b1b013c1ca859eaed66f1a Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Thu, 11 Jan 2024 16:57:28 +0100
Subject: [PATCH] evaluate: error out when expression has no datatype
add rule ip6 f i rt2 addr . ip6 daddr { dead:: . dead:: }
... will cause a segmentation fault, we assume expr->dtype is always
set.
rt2 support is incomplete, the template is uninitialised.
This could be fixed up, but rt2 (a subset of the deperecated type 0),
like all other routing headers, lacks correct dependency tracking.
Currently such routing headers are always assumed to be segment routing
headers, we would need to add dependency on 'Routing Type' field in the
routing header, similar to icmp type/code.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 41524eef..197c82c2 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1593,6 +1593,11 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
"cannot use %s in concatenation",
expr_name(i));
+ if (!i->dtype)
+ return expr_error(ctx->msgs, i,
+ "cannot use %s in concatenation, lacks datatype",
+ expr_name(i));
+
flags &= i->flags;
if (!key && i->dtype->type == TYPE_INTEGER) {
--
2.33.0

View File

@ -0,0 +1,54 @@
From 8a66de2a15943b2fbf960967cdbcbd0a148cb114 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Mon, 15 Jan 2024 14:11:17 +0100
Subject: [PATCH] evaluate: error out when store needs more than one 128bit
register of align fixup
Else this gives:
nft: evaluate.c:2983: stmt_evaluate_payload: Assertion `sizeof(data) * BITS_PER_BYTE >= masklen' failed.
For loads, this is already prevented via expr_evaluate_bits() which has:
if (masklen > NFT_REG_SIZE * BITS_PER_BYTE)
return expr_error(ctx->msgs, expr, "mask length %u exceeds allowed maximum of %u\n",
masklen, NFT_REG_SIZE * BITS_PER_BYTE);
But for the store path this isn't called.
The reproducer asks to store a 128 bit integer at bit offset 1, i.e.
17 bytes would need to be munged, but we can only handle up to 16 bytes
(one pseudo-register).
Fixes: 78936d50f306 ("evaluate: add support to set IPv6 non-byte header fields")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 5 +++++
.../testcases/bogons/nft-f/payload_expr_unaligned_store | 1 +
2 files changed, 6 insertions(+)
create mode 100644 tests/shell/testcases/bogons/nft-f/payload_expr_unaligned_store
diff --git a/src/evaluate.c b/src/evaluate.c
index 3b366166..68cfd776 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3188,6 +3188,11 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
payload_byte_size = div_round_up(payload->len + extra_len,
BITS_PER_BYTE);
+ if (payload_byte_size > sizeof(data))
+ return expr_error(ctx->msgs, stmt->payload.expr,
+ "uneven load cannot span more than %u bytes, got %u",
+ sizeof(data), payload_byte_size);
+
if (need_csum && payload_byte_size & 1) {
payload_byte_size++;
diff --git a/tests/shell/testcases/bogons/nft-f/payload_expr_unaligned_store b/tests/shell/testcases/bogons/nft-f/payload_expr_unaligned_store
new file mode 100644
index 00000000..c1358df4
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/payload_expr_unaligned_store
@@ -0,0 +1 @@
+add rule f i @th,1,128 set 1
--
2.33.0

View File

@ -0,0 +1,42 @@
From 7e6aa6db1fe5b14b5d224da11b077c50cc954efa Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 29 Aug 2023 14:53:33 +0200
Subject: [PATCH] evaluate: fix check for truncation in
stmt_evaluate_log_prefix()
Otherwise, nft crashes with prefix longer than 127 bytes:
# nft add rule x y log prefix \"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\"
==159385==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffed5bf4a10 at pc 0x7f3134839269 bp 0x7ffed5bf48b0 sp 0x7ffed5bf4060
WRITE of size 129 at 0x7ffed5bf4a10 thread T0
#0 0x7f3134839268 in __interceptor_memset ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:778
#1 0x7f3133e3074e in __mpz_export_data /tmp/nftables/src/gmputil.c:110
#2 0x7f3133d21d3c in expr_to_string /tmp/nftables/src/expression.c:192
#3 0x7f3133ded103 in netlink_gen_log_stmt /tmp/nftables/src/netlink_linearize.c:1148
#4 0x7f3133df33a1 in netlink_gen_stmt /tmp/nftables/src/netlink_linearize.c:1682
[...]
Fixes: e76bb3794018 ('src: allow for variables in the log prefix string')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index eb834eae..4c02a9cd 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4150,7 +4150,7 @@ static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
}
- if (len == NF_LOG_PREFIXLEN)
+ if (len == 0)
return stmt_error(ctx, stmt, "log prefix is too long");
expr = constant_expr_alloc(&stmt->log.prefix->location, &string_type,
--
2.33.0

View File

@ -0,0 +1,43 @@
From ee73e9dcd46dc5a1fe3be7caa8b9323819e394b8 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Tue, 5 Dec 2023 13:08:17 +0100
Subject: [PATCH] evaluate: fix double free on dtype release
We release ->dtype twice, will either segfault or assert
on dtype->refcount != 0 check in datatype_free().
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 2 +-
.../bogons/nft-f/double-free-on-binop-dtype_assert | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/double-free-on-binop-dtype_assert
diff --git a/src/evaluate.c b/src/evaluate.c
index 16ad6473..58cc811a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1171,7 +1171,7 @@ static int expr_evaluate_prefix(struct eval_ctx *ctx, struct expr **expr)
base = prefix->prefix;
assert(expr_is_constant(base));
- prefix->dtype = base->dtype;
+ prefix->dtype = datatype_get(base->dtype);
prefix->byteorder = base->byteorder;
prefix->len = base->len;
prefix->flags |= EXPR_F_CONSTANT;
diff --git a/tests/shell/testcases/bogons/nft-f/double-free-on-binop-dtype_assert b/tests/shell/testcases/bogons/nft-f/double-free-on-binop-dtype_assert
new file mode 100644
index 00000000..b7a9a1cc
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/double-free-on-binop-dtype_assert
@@ -0,0 +1,6 @@
+table inet t {
+ chain c {
+ udp length . @th,160,118 vmap { 47-63 . 0xe3731353631303331313037353532/3 : accept }
+ jump noexist # only here so this fails to load after patch.
+ }
+}
--
2.33.0

View File

@ -0,0 +1,60 @@
From fe727d5da18c40cb9f002eeaf0116f59e9600659 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 15 Sep 2023 02:30:27 +0200
Subject: [PATCH] evaluate: fix memleak in prefix evaluation with wildcard
interface name
The following ruleset:
table ip x {
chain y {
meta iifname { abcde*, xyz }
}
}
triggers the following memleak:
==6871== 16 bytes in 1 blocks are definitely lost in loss record 1 of 1
==6871== at 0x483877F: malloc (vg_replace_malloc.c:307)
==6871== by 0x48AD898: xmalloc (utils.c:37)
==6871== by 0x4BC8B22: __gmpz_init2 (in /usr/lib/x86_64-linux-gnu/libgmp.so.10.4.1)
==6871== by 0x4887E67: constant_expr_alloc (expression.c:424)
==6871== by 0x488EF1F: expr_evaluate_prefix (evaluate.c:1138)
==6871== by 0x488EF1F: expr_evaluate (evaluate.c:2725)
==6871== by 0x488E76D: expr_evaluate_set_elem (evaluate.c:1662)
==6871== by 0x488E76D: expr_evaluate (evaluate.c:2739)
==6871== by 0x4891033: list_member_evaluate (evaluate.c:1454)
==6871== by 0x488E2B6: expr_evaluate_set (evaluate.c:1757)
==6871== by 0x488E2B6: expr_evaluate (evaluate.c:2737)
==6871== by 0x48910D0: elems_evaluate (evaluate.c:4605)
==6871== by 0x4891432: set_evaluate (evaluate.c:4711)
==6871== by 0x48915BC: implicit_set_declaration (evaluate.c:122)
==6871== by 0x488F18A: expr_evaluate_relational (evaluate.c:2503)
==6871== by 0x488F18A: expr_evaluate (evaluate.c:2745)
expr_evaluate_prefix() calls constant_expr_alloc() which have already
called mpz_init2(), the second call to mpz_init2() overlaps the existing
mpz_t data memory area.
Remove extra mpz_init2() call to fix this memleak.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 1b7e0b37..90e7bff6 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1142,7 +1142,6 @@ static int expr_evaluate_prefix(struct eval_ctx *ctx, struct expr **expr)
mpz_prefixmask(mask->value, base->len, prefix->prefix_len);
break;
case TYPE_STRING:
- mpz_init2(mask->value, base->len);
mpz_bitmask(mask->value, prefix->prefix_len);
break;
}
--
2.33.0

View File

@ -0,0 +1,52 @@
From 4b6a4ad9134fa71277c2ff7f92776e1faeb83000 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 25 Oct 2023 16:00:50 +0200
Subject: [PATCH] evaluate: reject set in concatenation
Consider the following ruleset.
define ext_if = { "eth0", "eth1" }
table ip filter {
chain c {
iifname . tcp dport { $ext_if . 22 } accept
}
}
Attempting to load this ruleset results in:
BUG: invalid expression type 'set' in setnft: netlink.c:304: __netlink_gen_concat_key: Assertion `0' failed.
Aborted (core dumped)
After this patch:
# nft -f ruleset.nft
ruleset.nft:1:17-40: Error: cannot use set in concatenation
define ext_if = { "eth0", "eth1" }
^^^^^^^^^^^^^^^^^^
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1715
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/evaluate.c b/src/evaluate.c
index 2196e928..894987df 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1511,6 +1511,12 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
if (list_member_evaluate(ctx, &i) < 0)
return -1;
+
+ if (i->etype == EXPR_SET)
+ return expr_error(ctx->msgs, i,
+ "cannot use %s in concatenation",
+ expr_name(i));
+
flags &= i->flags;
if (!key && i->dtype->type == TYPE_INTEGER) {
--
2.33.0

View File

@ -0,0 +1,116 @@
From fa17b17ea74a21a44596f3212466ff3d2d3ede8e Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Sat, 2 Sep 2023 10:37:39 +0200
Subject: [PATCH] evaluate: revisit anonymous set with single element
optimization
This patch reworks it to perform this optimization from the evaluation
step of the relational expression. Hence, when optimizing for protocol
flags, use OP_EQ instead of OP_IMPLICIT, that is:
tcp flags { syn }
becomes (to represent an exact match):
tcp flags == syn
given OP_IMPLICIT and OP_EQ are not equivalent for flags.
01167c393a12 ("evaluate: do not remove anonymous set with protocol flags
and single element") disabled this optimization, which is enabled again
after this patch.
Fixes: 01167c393a12 ("evaluate: do not remove anonymous set with protocol flags and single element")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 60 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index a7725f4e..ab3ec987 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1815,26 +1815,6 @@ static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
if (ctx->set) {
if (ctx->set->flags & NFT_SET_CONCAT)
set->set_flags |= NFT_SET_CONCAT;
- } else if (set->size == 1) {
- i = list_first_entry(&set->expressions, struct expr, list);
- if (i->etype == EXPR_SET_ELEM &&
- (!i->dtype->basetype ||
- i->dtype->basetype->type != TYPE_BITMASK ||
- i->dtype->type == TYPE_CT_STATE) &&
- list_empty(&i->stmt_list)) {
-
- switch (i->key->etype) {
- case EXPR_PREFIX:
- case EXPR_RANGE:
- case EXPR_VALUE:
- *expr = i->key;
- i->key = NULL;
- expr_free(set);
- return 0;
- default:
- break;
- }
- }
}
set->set_flags |= NFT_SET_CONSTANT;
@@ -2355,6 +2335,35 @@ static bool range_needs_swap(const struct expr *range)
return mpz_cmp(left->value, right->value) > 0;
}
+static void optimize_singleton_set(struct expr *rel, struct expr **expr)
+{
+ struct expr *set = rel->right, *i;
+
+ i = list_first_entry(&set->expressions, struct expr, list);
+ if (i->etype == EXPR_SET_ELEM &&
+ list_empty(&i->stmt_list)) {
+
+ switch (i->key->etype) {
+ case EXPR_PREFIX:
+ case EXPR_RANGE:
+ case EXPR_VALUE:
+ rel->right = *expr = i->key;
+ i->key = NULL;
+ expr_free(set);
+ break;
+ default:
+ break;
+ }
+ }
+
+ if (rel->op == OP_IMPLICIT &&
+ rel->right->dtype->basetype &&
+ rel->right->dtype->basetype->type == TYPE_BITMASK &&
+ rel->right->dtype->type != TYPE_CT_STATE) {
+ rel->op = OP_EQ;
+ }
+}
+
static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr)
{
struct expr *rel = *expr, *left, *right;
@@ -2434,6 +2443,17 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr)
return expr_binary_error(ctx->msgs, right, left,
"Cannot be used with right hand side constant value");
+ switch (rel->op) {
+ case OP_EQ:
+ case OP_IMPLICIT:
+ case OP_NEQ:
+ if (right->etype == EXPR_SET && right->size == 1)
+ optimize_singleton_set(rel, &right);
+ break;
+ default:
+ break;
+ }
+
switch (rel->op) {
case OP_EQ:
case OP_IMPLICIT:
--
2.33.0

View File

@ -0,0 +1,49 @@
From 6bc6673fc88c8a3e3dd5504b2d24a6d6bc2f8427 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 10 Jan 2024 18:18:50 +0100
Subject: [PATCH] evaluate: skip anonymous set optimization for concatenations
Concatenation is only supported with sets. Moreover, stripping of the
set leads to broken ruleset listing, therefore, skip this optimization
for the concatenations.
Fixes: fa17b17ea74a ("evaluate: revisit anonymous set with single element optimization")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index b13e7c02..78732c6e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2580,15 +2580,17 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr)
return expr_binary_error(ctx->msgs, right, left,
"Cannot be used with right hand side constant value");
- switch (rel->op) {
- case OP_EQ:
- case OP_IMPLICIT:
- case OP_NEQ:
- if (right->etype == EXPR_SET && right->size == 1)
- optimize_singleton_set(rel, &right);
- break;
- default:
- break;
+ if (left->etype != EXPR_CONCAT) {
+ switch (rel->op) {
+ case OP_EQ:
+ case OP_IMPLICIT:
+ case OP_NEQ:
+ if (right->etype == EXPR_SET && right->size == 1)
+ optimize_singleton_set(rel, &right);
+ break;
+ default:
+ break;
+ }
}
switch (rel->op) {
--
2.33.0

View File

@ -0,0 +1,64 @@
From 3eb0a73a9ee32897290d4097c0ec29377e25859e Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Wed, 13 Dec 2023 17:00:37 +0100
Subject: [PATCH] evaluate: stmt_nat: set reference must point to a map
nat_concat_map() requires a datamap, else we crash:
set->data is dereferenced.
Also update expr_evaluate_map() so that EXPR_SET_REF is checked there
too.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 9 +++++++++
.../bogons/nft-f/nat_stmt_with_set_instead_of_map | 10 ++++++++++
2 files changed, 19 insertions(+)
create mode 100644 tests/shell/testcases/bogons/nft-f/nat_stmt_with_set_instead_of_map
diff --git a/src/evaluate.c b/src/evaluate.c
index 1b3e8097..da382912 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2041,6 +2041,9 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
break;
case EXPR_SET_REF:
/* symbol has been already evaluated to set reference */
+ if (!set_is_map(mappings->set->flags))
+ return expr_error(ctx->msgs, map->mappings,
+ "Expression is not a map");
break;
default:
BUG("invalid mapping expression %s\n",
@@ -3969,6 +3972,12 @@ static bool nat_concat_map(struct eval_ctx *ctx, struct stmt *stmt)
if (expr_evaluate(ctx, &stmt->nat.addr->mappings))
return false;
+ if (!set_is_datamap(stmt->nat.addr->mappings->set->flags)) {
+ expr_error(ctx->msgs, stmt->nat.addr->mappings,
+ "Expression is not a map");
+ return false;
+ }
+
if (stmt->nat.addr->mappings->set->data->etype == EXPR_CONCAT ||
stmt->nat.addr->mappings->set->data->dtype->subtypes) {
stmt->nat.type_flags |= STMT_NAT_F_CONCAT;
diff --git a/tests/shell/testcases/bogons/nft-f/nat_stmt_with_set_instead_of_map b/tests/shell/testcases/bogons/nft-f/nat_stmt_with_set_instead_of_map
new file mode 100644
index 00000000..b1302278
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/nat_stmt_with_set_instead_of_map
@@ -0,0 +1,10 @@
+table inet x {
+ set y {
+ type ipv4_addr
+ elements = { 2.2.2.2, 3.3.3.3 }
+ }
+
+ chain y {
+ snat ip to ip saddr map @y
+ }
+}
--
2.33.0

View File

@ -0,0 +1,69 @@
From 1d03ab5267bdbc7c0bcb041efaad42a462fdeb5f Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Thu, 11 Jan 2024 18:14:15 +0100
Subject: [PATCH] evaluate: tproxy: move range error checks after arg
evaluation
Testing for range before evaluation will still crash us later during
netlink linearization, prefixes turn into ranges, symbolic expression
might hide a range/prefix.
So move this after the argument has been evaluated.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 12 ++++++------
tests/shell/testcases/bogons/nft-f/tproxy_ranges | 8 ++++++++
2 files changed, 14 insertions(+), 6 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/tproxy_ranges
diff --git a/src/evaluate.c b/src/evaluate.c
index 197c82c2..d11bed01 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4132,22 +4132,22 @@ static int stmt_evaluate_tproxy(struct eval_ctx *ctx, struct stmt *stmt)
return err;
if (stmt->tproxy.addr != NULL) {
- if (stmt->tproxy.addr->etype == EXPR_RANGE)
- return stmt_error(ctx, stmt, "Address ranges are not supported for tproxy.");
-
err = stmt_evaluate_addr(ctx, stmt, &stmt->tproxy.family,
&stmt->tproxy.addr);
-
if (err < 0)
return err;
+
+ if (stmt->tproxy.addr->etype == EXPR_RANGE)
+ return stmt_error(ctx, stmt, "Address ranges are not supported for tproxy.");
}
if (stmt->tproxy.port != NULL) {
- if (stmt->tproxy.port->etype == EXPR_RANGE)
- return stmt_error(ctx, stmt, "Port ranges are not supported for tproxy.");
err = nat_evaluate_transport(ctx, stmt, &stmt->tproxy.port);
if (err < 0)
return err;
+
+ if (stmt->tproxy.port->etype == EXPR_RANGE)
+ return stmt_error(ctx, stmt, "Port ranges are not supported for tproxy.");
}
return 0;
diff --git a/tests/shell/testcases/bogons/nft-f/tproxy_ranges b/tests/shell/testcases/bogons/nft-f/tproxy_ranges
new file mode 100644
index 00000000..1230860e
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/tproxy_ranges
@@ -0,0 +1,8 @@
+define range = 42-80
+
+table t {
+ chain c {
+ tcp dport 42 tproxy to 192.168.0.1:$range
+ tcp dport 42 tproxy to 192.168.0.0/16
+ }
+}
--
2.33.0

View File

@ -0,0 +1,105 @@
From 08925ba0daf19753df933fed69f4572a7c9d3d47 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Sat, 9 Dec 2023 00:37:09 +0100
Subject: [PATCH] evaluate: validate chain max length
The includes test files cause:
BUG: chain is too large (257, 256 max)nft: netlink.c:418: netlink_gen_chain: Assertion `0' failed.
Error out in evaluation step instead.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 34 ++++++++++++++++++-
.../bogons/nft-f/huge_chain_name_assert | 5 +++
.../nft-f/huge_chain_name_define_assert | 7 ++++
3 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/huge_chain_name_assert
create mode 100644 tests/shell/testcases/bogons/nft-f/huge_chain_name_define_assert
diff --git a/src/evaluate.c b/src/evaluate.c
index a62a2346..715c398a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2738,6 +2738,35 @@ static int expr_evaluate_flagcmp(struct eval_ctx *ctx, struct expr **exprp)
return expr_evaluate(ctx, exprp);
}
+static int verdict_validate_chainlen(struct eval_ctx *ctx,
+ struct expr *chain)
+{
+ if (chain->len > NFT_CHAIN_MAXNAMELEN * BITS_PER_BYTE)
+ return expr_error(ctx->msgs, chain,
+ "chain name too long (%u, max %u)",
+ chain->len / BITS_PER_BYTE,
+ NFT_CHAIN_MAXNAMELEN);
+
+ return 0;
+}
+
+static int expr_evaluate_verdict(struct eval_ctx *ctx, struct expr **exprp)
+{
+ struct expr *expr = *exprp;
+
+ switch (expr->verdict) {
+ case NFT_GOTO:
+ case NFT_JUMP:
+ if (expr->chain->etype == EXPR_VALUE &&
+ verdict_validate_chainlen(ctx, expr->chain))
+ return -1;
+
+ break;
+ }
+
+ return expr_evaluate_primary(ctx, exprp);
+}
+
static int expr_evaluate(struct eval_ctx *ctx, struct expr **expr)
{
if (ctx->nft->debug_mask & NFT_DEBUG_EVALUATION) {
@@ -2763,7 +2792,7 @@ static int expr_evaluate(struct eval_ctx *ctx, struct expr **expr)
case EXPR_EXTHDR:
return expr_evaluate_exthdr(ctx, expr);
case EXPR_VERDICT:
- return expr_evaluate_primary(ctx, expr);
+ return expr_evaluate_verdict(ctx, expr);
case EXPR_META:
return expr_evaluate_meta(ctx, expr);
case EXPR_SOCKET:
@@ -2964,6 +2993,9 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
return expr_error(ctx->msgs, stmt->expr->chain,
"not a value expression");
}
+
+ if (verdict_validate_chainlen(ctx, stmt->expr->chain))
+ return -1;
}
break;
case EXPR_MAP:
diff --git a/tests/shell/testcases/bogons/nft-f/huge_chain_name_assert b/tests/shell/testcases/bogons/nft-f/huge_chain_name_assert
new file mode 100644
index 00000000..161f867d
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/huge_chain_name_assert
@@ -0,0 +1,5 @@
+table inet x {
+ chain c {
+ udp length vmap { 1 : goto rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr }
+ }
+}
diff --git a/tests/shell/testcases/bogons/nft-f/huge_chain_name_define_assert b/tests/shell/testcases/bogons/nft-f/huge_chain_name_define_assert
new file mode 100644
index 00000000..3c2c0d3e
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/huge_chain_name_define_assert
@@ -0,0 +1,7 @@
+define huge = rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr
+
+table t {
+ chain d {
+ jump $huge
+ }
+}
--
2.33.0

View File

@ -0,0 +1,36 @@
From 6ceec21204e0260af2d50e9e987d0fe3c79c28d4 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 17 Oct 2023 15:50:21 +0200
Subject: [PATCH] evaluate: validate maximum log statement prefix length
Otherwise too long string overruns the log prefix buffer.
Fixes: e76bb3794018 ("src: allow for variables in the log prefix string")
Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1714
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/evaluate.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index b7ae9113..2196e928 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4175,8 +4175,13 @@ static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
struct expr *expr;
size_t size = 0;
- if (stmt->log.prefix->etype != EXPR_LIST)
+ if (stmt->log.prefix->etype != EXPR_LIST) {
+ if (stmt->log.prefix &&
+ div_round_up(stmt->log.prefix->len, BITS_PER_BYTE) >= NF_LOG_PREFIXLEN)
+ return expr_error(ctx->msgs, stmt->log.prefix, "log prefix is too long");
+
return 0;
+ }
list_for_each_entry(expr, &stmt->log.prefix->expressions, list) {
switch (expr->etype) {
--
2.33.0

View File

@ -0,0 +1,40 @@
From 574fa55c2e70449887c7714d7b043f4e8b6d28da Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Fri, 14 Jul 2023 16:53:57 +0200
Subject: [PATCH] exthdr: prefer raw_type instead of desc->type
On ancient kernels desc can be NULL, because such kernels do not
understand NFTA_EXTHDR_TYPE.
Thus they don't include it in the reverse dump, so the tcp/ip
option gets treated like an ipv6 exthdr, but no matching
description will be found.
This then gives a crash due to the null deref.
Just use the raw value here, this avoid a crash and at least
print *something*, e.g.:
unknown-exthdr unknown & 0xf0 [invalid type] == 0x0 [invalid type]
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/exthdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/exthdr.c b/src/exthdr.c
index f5527ddb..0358005b 100644
--- a/src/exthdr.c
+++ b/src/exthdr.c
@@ -405,7 +405,7 @@ bool exthdr_find_template(struct expr *expr, const struct expr *mask, unsigned i
found = tcpopt_find_template(expr, off, mask_len - mask_offset);
break;
case NFT_EXTHDR_OP_IPV6:
- exthdr_init_raw(expr, expr->exthdr.desc->type,
+ exthdr_init_raw(expr, expr->exthdr.raw_type,
off, mask_len - mask_offset, expr->exthdr.op, 0);
/* still failed to find a template... Bug. */
--
2.33.0

View File

@ -0,0 +1,45 @@
From 57f5aca0006ebf984ffc1f66d48cf3b74a3d1f59 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 26 Sep 2023 16:55:50 +0200
Subject: [PATCH] json: expose dynamic flag
The dynamic flag is not exported via JSON, this triggers spurious
ENOTSUPP errors when restoring rulesets in JSON with dynamic flags
set on.
Fixes: 6e45b102650a2 ("nft: set: print dynamic flag when set")
Acked-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/json.c | 2 ++
src/parser_json.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/json.c b/src/json.c
index 446575c2..220ce0f7 100644
--- a/src/json.c
+++ b/src/json.c
@@ -176,6 +176,8 @@ static json_t *set_print_json(struct output_ctx *octx, const struct set *set)
json_array_append_new(tmp, json_pack("s", "interval"));
if (set->flags & NFT_SET_TIMEOUT)
json_array_append_new(tmp, json_pack("s", "timeout"));
+ if (set->flags & NFT_SET_EVAL)
+ json_array_append_new(tmp, json_pack("s", "dynamic"));
if (json_array_size(tmp) > 0) {
json_object_set_new(root, "flags", tmp);
diff --git a/src/parser_json.c b/src/parser_json.c
index df327e95..16961d60 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3136,6 +3136,7 @@ static int string_to_set_flag(const char *str)
{ NFT_SET_CONSTANT, "constant" },
{ NFT_SET_INTERVAL, "interval" },
{ NFT_SET_TIMEOUT, "timeout" },
+ { NFT_SET_EVAL, "dynamic" },
};
unsigned int i;
--
2.33.0

View File

@ -0,0 +1,49 @@
From b04512cf30de1ba6657facba5ebe2321e17c2727 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 14 Nov 2023 16:29:25 +0100
Subject: [PATCH] json: fix use after free in table_flags_json()
Add `$NFT -j list ruleset` to the end of "tests/shell/testcases/transactions/table_onoff".
Then valgrind will find this issue:
$ make -j && ./tests/shell/run-tests.sh tests/shell/testcases/transactions/table_onoff -V
Gives:
==286== Invalid read of size 4
==286== at 0x49B0261: do_dump (dump.c:211)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B04F7: do_dump (dump.c:273)
==286== by 0x49B08B8: do_dump (dump.c:378)
==286== by 0x49B0E84: json_dump_callback (dump.c:465)
==286== by 0x48AF22A: do_command_list_json (json.c:2016)
==286== by 0x48732F1: do_command_list (rule.c:2335)
==286== by 0x48737F5: do_command (rule.c:2605)
==286== by 0x48A867D: nft_netlink (libnftables.c:42)
==286== by 0x48A92B1: nft_run_cmd_from_buffer (libnftables.c:597)
==286== by 0x402CBA: main (main.c:533)
Fixes: e70354f53e9f ("libnftables: Implement JSON output support")
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/json.c b/src/json.c
index 23bd2472..81328ab3 100644
--- a/src/json.c
+++ b/src/json.c
@@ -496,7 +496,7 @@ static json_t *table_flags_json(const struct table *table)
json_decref(root);
return NULL;
case 1:
- json_unpack(root, "[o]", &tmp);
+ json_unpack(root, "[O]", &tmp);
json_decref(root);
root = tmp;
break;
--
2.33.0

View File

@ -0,0 +1,100 @@
From 458e91a954abe4b7fb4ba70901c7da28220c446a Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 31 Jul 2023 12:29:55 +0200
Subject: [PATCH] libnftables: Drop cache in -c/--check mode
Extend e0aace943412 ("libnftables: Drop cache in error case") to also
drop the cache with -c/--check, this is a dry run mode and kernel does
not get any update.
This fixes a bug with -o/--optimize, which first runs in an implicit
-c/--check mode to validate that the ruleset is correct, then it
provides the proposed optimization. In this case, if the cache is not
emptied, old objects in the cache refer to scanner data that was
already released, which triggers BUG like this:
BUG: invalid input descriptor type 151665524
nft: erec.c:161: erec_print: Assertion `0' failed.
Aborted
This bug was triggered in a ruleset that contains a set for geoip
filtering. This patch also extends tests/shell to cover this case.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/libnftables.c | 7 +++++--
.../optimizations/dumps/skip_unsupported.nft | 11 +++++++++++
tests/shell/testcases/optimizations/skip_unsupported | 11 +++++++++++
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/libnftables.c b/src/libnftables.c
index 6fc4f7db..e214abb6 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -607,8 +607,10 @@ err:
nft_output_json(&nft->output) &&
nft_output_echo(&nft->output))
json_print_echo(nft);
- if (rc)
+
+ if (rc || nft->check)
nft_cache_release(&nft->cache);
+
return rc;
}
@@ -713,7 +715,8 @@ err:
nft_output_json(&nft->output) &&
nft_output_echo(&nft->output))
json_print_echo(nft);
- if (rc)
+
+ if (rc || nft->check)
nft_cache_release(&nft->cache);
scope_release(nft->state->scopes[0]);
diff --git a/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft b/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
index 43b6578d..f24855e7 100644
--- a/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
+++ b/tests/shell/testcases/optimizations/dumps/skip_unsupported.nft
@@ -1,4 +1,15 @@
table inet x {
+ set GEOIP_CC_wan-lan_120 {
+ type ipv4_addr
+ flags interval
+ elements = { 1.32.128.0/18, 1.32.200.0-1.32.204.128,
+ 1.32.207.0/24, 1.32.216.118-1.32.216.255,
+ 1.32.219.0-1.32.222.255, 1.32.226.0/23,
+ 1.32.231.0/24, 1.32.233.0/24,
+ 1.32.238.0/23, 1.32.240.0/24,
+ 223.223.220.0/22, 223.255.254.0/24 }
+ }
+
chain y {
ip saddr 1.2.3.4 tcp dport 80 meta mark set 0x0000000a accept
ip saddr 1.2.3.4 tcp dport 81 meta mark set 0x0000000b accept
diff --git a/tests/shell/testcases/optimizations/skip_unsupported b/tests/shell/testcases/optimizations/skip_unsupported
index 9313c302..6baa8280 100755
--- a/tests/shell/testcases/optimizations/skip_unsupported
+++ b/tests/shell/testcases/optimizations/skip_unsupported
@@ -3,6 +3,17 @@
set -e
RULESET="table inet x {
+ set GEOIP_CC_wan-lan_120 {
+ type ipv4_addr
+ flags interval
+ elements = { 1.32.128.0/18, 1.32.200.0-1.32.204.128,
+ 1.32.207.0/24, 1.32.216.118-1.32.216.255,
+ 1.32.219.0-1.32.222.255, 1.32.226.0/23,
+ 1.32.231.0/24, 1.32.233.0/24,
+ 1.32.238.0/23, 1.32.240.0/24,
+ 223.223.220.0/22, 223.255.254.0/24 }
+ }
+
chain y {
ip saddr 1.2.3.4 tcp dport 80 meta mark set 10 accept
ip saddr 1.2.3.4 tcp dport 81 meta mark set 11 accept
--
2.33.0

View File

@ -0,0 +1,104 @@
From 7008b1200fb4988b7cd7ee1c5399cae071688d50 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Wed, 13 Dec 2023 17:37:11 +0100
Subject: [PATCH] meta: fix tc classid parsing out-of-bounds access
AddressSanitizer: heap-buffer-overflow on address 0x6020000003af ...
#0 0x7f9a83cbb402 in tchandle_type_parse src/meta.c:89
#1 0x7f9a83c6753f in symbol_parse src/datatype.c:138
strlen() - 1 can underflow if length was 0.
Simplify the function, there is no need to duplicate the string
while scanning it.
Expect the first strtol to stop at ':', scan for the minor number next.
The second scan is required to stop at '\0'.
Fixes: 6f2eb8548e0d ("src: meta priority support using tc classid")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/meta.c | 29 ++++++-------------
.../nft-f/tchandle_type_parse_heap_overflow | 6 ++++
2 files changed, 15 insertions(+), 20 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/tchandle_type_parse_heap_overflow
diff --git a/src/meta.c b/src/meta.c
index d7f810ce..8d0b7aae 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -62,50 +62,39 @@ static struct error_record *tchandle_type_parse(struct parse_ctx *ctx,
struct expr **res)
{
uint32_t handle;
- char *str = NULL;
if (strcmp(sym->identifier, "root") == 0)
handle = TC_H_ROOT;
else if (strcmp(sym->identifier, "none") == 0)
handle = TC_H_UNSPEC;
else if (strchr(sym->identifier, ':')) {
+ char *colon, *end;
uint32_t tmp;
- char *colon;
-
- str = xstrdup(sym->identifier);
-
- colon = strchr(str, ':');
- if (!colon)
- goto err;
-
- *colon = '\0';
errno = 0;
- tmp = strtoull(str, NULL, 16);
- if (errno != 0)
+ tmp = strtoul(sym->identifier, &colon, 16);
+ if (errno != 0 || sym->identifier == colon)
goto err;
- handle = (tmp << 16);
- if (str[strlen(str) - 1] == ':')
- goto out;
+ if (*colon != ':')
+ goto err;
+ handle = tmp << 16;
errno = 0;
- tmp = strtoull(colon + 1, NULL, 16);
- if (errno != 0)
+ tmp = strtoul(colon + 1, &end, 16);
+ if (errno != 0 || *end)
goto err;
handle |= tmp;
} else {
handle = strtoull(sym->identifier, NULL, 0);
}
-out:
- xfree(str);
+
*res = constant_expr_alloc(&sym->location, sym->dtype,
BYTEORDER_HOST_ENDIAN,
sizeof(handle) * BITS_PER_BYTE, &handle);
return NULL;
err:
- xfree(str);
return error(&sym->location, "Could not parse %s", sym->dtype->desc);
}
diff --git a/tests/shell/testcases/bogons/nft-f/tchandle_type_parse_heap_overflow b/tests/shell/testcases/bogons/nft-f/tchandle_type_parse_heap_overflow
new file mode 100644
index 00000000..ea7186bf
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/tchandle_type_parse_heap_overflow
@@ -0,0 +1,6 @@
+table t {
+map m {
+ type ipv4_addr : classid
+ elements = { 1.1.26.3 : ::a }
+}
+}
--
2.33.0

View File

@ -0,0 +1,148 @@
From 0404ff08b3c18052e6689d75fa85275d3cef7e8e Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Thu, 14 Dec 2023 15:39:27 +0100
Subject: [PATCH] netlink: don't crash if prefix for < byte is requested
If prefix is used with a datatype that has less than 8 bits an
assertion is triggered:
src/netlink.c:243: netlink_gen_raw_data: Assertion `len > 0' failed.
This is esoteric, the alternative would be to restrict prefixes
to ipv4/ipv6 addresses.
Simpler fix is to use round_up instead of divide.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/netlink_linearize.c | 3 ++-
tests/py/ip/ip.t | 2 ++
tests/py/ip/ip.t.json | 21 +++++++++++++++++++++
tests/py/ip/ip.t.payload | 8 ++++++++
tests/py/ip/ip.t.payload.bridge | 10 ++++++++++
tests/py/ip/ip.t.payload.inet | 10 ++++++++++
tests/py/ip/ip.t.payload.netdev | 10 ++++++++++
7 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 61828eb9..d8b41a08 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -460,7 +460,8 @@ static struct expr *netlink_gen_prefix(struct netlink_linearize_ctx *ctx,
mpz_init(mask);
mpz_prefixmask(mask, expr->right->len, expr->right->prefix_len);
netlink_gen_raw_data(mask, expr->right->byteorder,
- expr->right->len / BITS_PER_BYTE, &nld);
+ div_round_up(expr->right->len, BITS_PER_BYTE),
+ &nld);
mpz_clear(mask);
zero.len = nld.len;
diff --git a/tests/py/ip/ip.t b/tests/py/ip/ip.t
index 720d9ae9..e6999c29 100644
--- a/tests/py/ip/ip.t
+++ b/tests/py/ip/ip.t
@@ -133,3 +133,5 @@ ip saddr . ip daddr vmap { 192.168.5.1-192.168.5.128 . 192.168.6.1-192.168.6.128
ip saddr . ip daddr { 192.0.2.1 . 10.0.0.1-10.0.0.2 };ok
ip saddr . ip daddr vmap { 192.168.5.1-192.168.5.128 . 192.168.6.1-192.168.6.128 : accept };ok
+
+ip dscp 1/6;ok;ip dscp & 0x3f == lephb
diff --git a/tests/py/ip/ip.t.json b/tests/py/ip/ip.t.json
index 882c94eb..a170e5c1 100644
--- a/tests/py/ip/ip.t.json
+++ b/tests/py/ip/ip.t.json
@@ -1809,3 +1809,23 @@
}
]
+# ip dscp 1/6
+[
+ {
+ "match": {
+ "left": {
+ "&": [
+ {
+ "payload": {
+ "field": "dscp",
+ "protocol": "ip"
+ }
+ },
+ 63
+ ]
+ },
+ "op": "==",
+ "right": "lephb"
+ }
+ }
+]
diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
index 43605a36..d7ddf7be 100644
--- a/tests/py/ip/ip.t.payload
+++ b/tests/py/ip/ip.t.payload
@@ -556,3 +556,11 @@ ip test-ip4 input
[ payload load 4b @ network header + 12 => reg 1 ]
[ payload load 4b @ network header + 16 => reg 9 ]
[ lookup reg 1 set __map%d dreg 0 ]
+
+# ip dscp 1/6
+ip test-ip4 input
+ [ payload load 1b @ network header + 1 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+ [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000003f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000001 ]
diff --git a/tests/py/ip/ip.t.payload.bridge b/tests/py/ip/ip.t.payload.bridge
index e506f300..53f881d3 100644
--- a/tests/py/ip/ip.t.payload.bridge
+++ b/tests/py/ip/ip.t.payload.bridge
@@ -726,3 +726,12 @@ bridge test-bridge input
[ payload load 4b @ network header + 16 => reg 9 ]
[ lookup reg 1 set __map%d dreg 0 ]
+# ip dscp 1/6
+bridge test-bridge input
+ [ meta load protocol => reg 1 ]
+ [ cmp eq reg 1 0x00000008 ]
+ [ payload load 1b @ network header + 1 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+ [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000003f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000001 ]
diff --git a/tests/py/ip/ip.t.payload.inet b/tests/py/ip/ip.t.payload.inet
index a7fa0faf..08674c98 100644
--- a/tests/py/ip/ip.t.payload.inet
+++ b/tests/py/ip/ip.t.payload.inet
@@ -726,3 +726,12 @@ inet test-inet input
[ payload load 4b @ network header + 16 => reg 9 ]
[ lookup reg 1 set __map%d dreg 0 ]
+# ip dscp 1/6
+inet test-inet input
+ [ meta load nfproto => reg 1 ]
+ [ cmp eq reg 1 0x00000002 ]
+ [ payload load 1b @ network header + 1 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+ [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000003f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000001 ]
diff --git a/tests/py/ip/ip.t.payload.netdev b/tests/py/ip/ip.t.payload.netdev
index aebd9d64..8220b05d 100644
--- a/tests/py/ip/ip.t.payload.netdev
+++ b/tests/py/ip/ip.t.payload.netdev
@@ -726,3 +726,12 @@ netdev test-netdev ingress
[ payload load 4b @ network header + 16 => reg 9 ]
[ lookup reg 1 set __map%d dreg 0 ]
+# ip dscp 1/6
+netdev test-netdev ingress
+ [ meta load protocol => reg 1 ]
+ [ cmp eq reg 1 0x00000008 ]
+ [ payload load 1b @ network header + 1 => reg 1 ]
+ [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+ [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+ [ bitwise reg 1 = ( reg 1 & 0x0000003f ) ^ 0x00000000 ]
+ [ cmp eq reg 1 0x00000001 ]
--
2.33.0

View File

@ -0,0 +1,79 @@
From a76c8960905ef6803c0c79df7bc0a030209c9455 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 14 Sep 2023 16:09:50 +0200
Subject: [PATCH] netlink: fix leaking typeof_expr_data/typeof_expr_key in
netlink_delinearize_set()
There are various code paths that return without freeing typeof_expr_data
and typeof_expr_key. It's not at all obvious, that there isn't a leak
that way. Quite possibly there is a leak. Fix it, or at least make the
code more obviously correct.
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/netlink.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/netlink.c b/src/netlink.c
index 4d3c1cf1..2489e986 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -937,12 +937,13 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
const struct nftnl_udata *ud[NFTNL_UDATA_SET_MAX + 1] = {};
enum byteorder keybyteorder = BYTEORDER_INVALID;
enum byteorder databyteorder = BYTEORDER_INVALID;
- struct expr *typeof_expr_key, *typeof_expr_data;
struct setelem_parse_ctx set_parse_ctx;
const struct datatype *datatype = NULL;
const struct datatype *keytype = NULL;
const struct datatype *dtype2 = NULL;
const struct datatype *dtype = NULL;
+ struct expr *typeof_expr_data = NULL;
+ struct expr *typeof_expr_key = NULL;
const char *udata, *comment = NULL;
uint32_t flags, key, objtype = 0;
uint32_t data_interval = 0;
@@ -951,9 +952,6 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
uint32_t ulen;
uint32_t klen;
- typeof_expr_key = NULL;
- typeof_expr_data = NULL;
-
if (nftnl_set_is_set(nls, NFTNL_SET_USERDATA)) {
udata = nftnl_set_get_data(nls, NFTNL_SET_USERDATA, &ulen);
if (nftnl_udata_parse(udata, ulen, set_parse_udata_cb, ud) < 0) {
@@ -1043,8 +1041,8 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
if (set_udata_key_valid(typeof_expr_data, dlen)) {
typeof_expr_data->len = klen;
set->data = typeof_expr_data;
+ typeof_expr_data = NULL;
} else {
- expr_free(typeof_expr_data);
set->data = constant_expr_alloc(&netlink_location,
dtype2,
databyteorder, klen,
@@ -1064,9 +1062,9 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
if (set_udata_key_valid(typeof_expr_key, klen)) {
set->key = typeof_expr_key;
+ typeof_expr_key = NULL;
set->key_typeof_valid = true;
} else {
- expr_free(typeof_expr_key);
set->key = constant_expr_alloc(&netlink_location, dtype,
keybyteorder, klen,
NULL);
@@ -1100,6 +1098,8 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
}
out:
+ expr_free(typeof_expr_data);
+ expr_free(typeof_expr_key);
datatype_free(datatype);
datatype_free(keytype);
datatype_free(dtype2);
--
2.33.0

View File

@ -0,0 +1,31 @@
From c4186c5376ee73efff005dbd23dd73a8e06e6ad8 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 20 Sep 2023 16:26:07 +0200
Subject: [PATCH] netlink: handle invalid etype in set_make_key()
It's not clear to me, what ensures that the etype is always valid.
Handle a NULL.
Fixes: 6e48df5329ea ('src: add "typeof" build/parse/print support')
Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
src/netlink.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/netlink.c b/src/netlink.c
index 2489e986..70ebf382 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -896,6 +896,8 @@ static struct expr *set_make_key(const struct nftnl_udata *attr)
etype = nftnl_udata_get_u32(ud[NFTNL_UDATA_SET_TYPEOF_EXPR]);
ops = expr_ops_by_type(etype);
+ if (!ops)
+ return NULL;
expr = ops->parse_udata(ud[NFTNL_UDATA_SET_TYPEOF_DATA]);
if (!expr)
--
2.33.0

View File

@ -0,0 +1,109 @@
From 037d58a27d675802286aafb23e409b8c1d3eef56 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Tue, 12 Dec 2023 10:22:58 +0100
Subject: [PATCH] parser_bison: fix ct scope underflow if ct helper section is
duplicated
table inet filter {
ct helper sip-5060u {
type "sip" protocol udp
l3proto ip
}5060t {
type "sip" protocol tcp
l3pownerip
}
Will close the 'ct' scope twice, it has to be closed AFTER the separator
has been parsed.
While not strictly needed, also error out if the protocol is already
given, this provides a better error description.
Also make sure we release the string in all error branches.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/parser_bison.y | 15 +++++++++++----
.../bogons/nft-f/ct_helper_yystate_underflow | 14 ++++++++++++++
2 files changed, 25 insertions(+), 4 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/ct_helper_yystate_underflow
diff --git a/src/parser_bison.y b/src/parser_bison.y
index d13fb961..ce80bcd9 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1971,7 +1971,7 @@ table_block : /* empty */ { $$ = $<table>-1; }
list_add_tail(&$4->list, &$1->objs);
$$ = $1;
}
- | table_block CT HELPER obj_identifier obj_block_alloc '{' ct_helper_block '}' close_scope_ct stmt_separator
+ | table_block CT HELPER obj_identifier obj_block_alloc '{' ct_helper_block '}' stmt_separator close_scope_ct
{
$5->location = @4;
$5->type = NFT_OBJECT_CT_HELPER;
@@ -1980,7 +1980,7 @@ table_block : /* empty */ { $$ = $<table>-1; }
list_add_tail(&$5->list, &$1->objs);
$$ = $1;
}
- | table_block CT TIMEOUT obj_identifier obj_block_alloc '{' ct_timeout_block '}' close_scope_ct stmt_separator
+ | table_block CT TIMEOUT obj_identifier obj_block_alloc '{' ct_timeout_block '}' stmt_separator close_scope_ct
{
$5->location = @4;
$5->type = NFT_OBJECT_CT_TIMEOUT;
@@ -1989,7 +1989,7 @@ table_block : /* empty */ { $$ = $<table>-1; }
list_add_tail(&$5->list, &$1->objs);
$$ = $1;
}
- | table_block CT EXPECTATION obj_identifier obj_block_alloc '{' ct_expect_block '}' close_scope_ct stmt_separator
+ | table_block CT EXPECTATION obj_identifier obj_block_alloc '{' ct_expect_block '}' stmt_separator close_scope_ct
{
$5->location = @4;
$5->type = NFT_OBJECT_CT_EXPECT;
@@ -4827,16 +4827,23 @@ ct_l4protoname : TCP close_scope_tcp { $$ = IPPROTO_TCP; }
| UDP close_scope_udp { $$ = IPPROTO_UDP; }
;
-ct_helper_config : TYPE QUOTED_STRING PROTOCOL ct_l4protoname stmt_separator close_scope_type
+ct_helper_config : TYPE QUOTED_STRING PROTOCOL ct_l4protoname stmt_separator close_scope_type
{
struct ct_helper *ct;
int ret;
ct = &$<obj>0->ct_helper;
+ if (ct->l4proto) {
+ erec_queue(error(&@2, "You can only specify this once. This statement is already set for %s.", ct->name), state->msgs);
+ xfree($2);
+ YYERROR;
+ }
+
ret = snprintf(ct->name, sizeof(ct->name), "%s", $2);
if (ret <= 0 || ret >= (int)sizeof(ct->name)) {
erec_queue(error(&@2, "invalid name '%s', max length is %u\n", $2, (int)sizeof(ct->name)), state->msgs);
+ xfree($2);
YYERROR;
}
xfree($2);
diff --git a/tests/shell/testcases/bogons/nft-f/ct_helper_yystate_underflow b/tests/shell/testcases/bogons/nft-f/ct_helper_yystate_underflow
new file mode 100644
index 00000000..18eb25eb
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/ct_helper_yystate_underflow
@@ -0,0 +1,14 @@
+table inet filter {
+ ct helper sip-5060u {
+ type "sip" protocol udp
+ l3proto ip
+ }5060t {
+ type "sip" protocol tcp
+ l3pownerip
+ }
+
+ chain input {
+ type filtol/dev/stdinok input priority f)lser; policy accept;
+ ct helper set ip protocol . th dport map { udp . 1-20000 : "si60u", tcp . 10000-20000 : "sip-5060t" }
+ }
+}
--
2.33.0

View File

@ -0,0 +1,41 @@
From 21608263cc1ae489326e743957bfe34b05414a44 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Fri, 8 Dec 2023 13:37:27 +0100
Subject: [PATCH] parser_bison: fix memleak in meta set error handling
We must release the expression here, found via afl++ and
-fsanitize-address build.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/parser_bison.y | 1 +
.../shell/testcases/bogons/nft-f/memleak_on_meta_set_errpath | 5 +++++
2 files changed, 6 insertions(+)
create mode 100644 tests/shell/testcases/bogons/nft-f/memleak_on_meta_set_errpath
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 64946a43..70acfc57 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5331,6 +5331,7 @@ meta_stmt : META meta_key SET stmt_expr close_scope_meta
xfree($2);
if (erec != NULL) {
erec_queue(erec, state->msgs);
+ expr_free($4);
YYERROR;
}
diff --git a/tests/shell/testcases/bogons/nft-f/memleak_on_meta_set_errpath b/tests/shell/testcases/bogons/nft-f/memleak_on_meta_set_errpath
new file mode 100644
index 00000000..917e8bf8
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/memleak_on_meta_set_errpath
@@ -0,0 +1,5 @@
+table filter {
+ chain y {
+ meta seccark set ct secmark
+ }
+}
--
2.33.0

View File

@ -0,0 +1,43 @@
From d5a06af393eaf47571c884a265d1f6e6ba34ed97 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Tue, 12 Dec 2023 10:44:35 +0100
Subject: [PATCH] parser_bison: make sure obj_free releases timeout policies
obj_free() won't release them because ->type is still 0 at this
point.
Init this to CT_TIMEOUT.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/parser_bison.y | 1 +
.../shell/testcases/bogons/nft-f/ct_timeout_memleak_objfree | 5 +++++
2 files changed, 6 insertions(+)
create mode 100644 tests/shell/testcases/bogons/nft-f/ct_timeout_memleak_objfree
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 70acfc57..d13fb961 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2513,6 +2513,7 @@ ct_timeout_block : /*empty */
{
$$ = $<obj>-1;
init_list_head(&$$->ct_timeout.timeout_list);
+ $$->type = NFT_OBJECT_CT_TIMEOUT;
}
| ct_timeout_block common_block
| ct_timeout_block stmt_separator
diff --git a/tests/shell/testcases/bogons/nft-f/ct_timeout_memleak_objfree b/tests/shell/testcases/bogons/nft-f/ct_timeout_memleak_objfree
new file mode 100644
index 00000000..28b1a211
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/ct_timeout_memleak_objfree
@@ -0,0 +1,5 @@
+table ip filter {
+ ct timeout cttime {
+ protocol tcp
+ l3proto ip
+ policy = { close : 12s }
--
2.33.0

View File

@ -0,0 +1,44 @@
From 7df0b2f1a1c64e2bdc652fd2418b4f7218c93f1f Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 13 Sep 2023 22:07:46 +0200
Subject: [PATCH] parser_json: Catch nonsense ops in match statement
Since expr_op_symbols array includes binary operators and more, simply
checking the given string matches any of the elements is not sufficient.
Fixes: 586ad210368b7 ("libnftables: Implement JSON parser")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index eec73034..c4a09797 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1725,13 +1725,18 @@ static struct stmt *json_parse_match_stmt(struct json_ctx *ctx,
!strcmp(opstr, expr_op_symbols[op]))
break;
}
- if (op == __OP_MAX) {
+ switch (op) {
+ case OP_EQ ... OP_NEG:
+ break;
+ case __OP_MAX:
if (!strcmp(opstr, "in")) {
op = OP_IMPLICIT;
- } else {
- json_error(ctx, "Unknown relational op '%s'.", opstr);
- return NULL;
+ break;
}
+ /* fall through */
+ default:
+ json_error(ctx, "Invalid relational op '%s'.", opstr);
+ return NULL;
}
left = json_parse_expr(ctx, jleft);
--
2.33.0

View File

@ -0,0 +1,30 @@
From 343006f5e0a6cdf907877218a354505dcc9d9864 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 20 Sep 2023 17:16:33 +0200
Subject: [PATCH] parser_json: Default meter size to zero
JSON parser was missed when performing the same change in standard
syntax parser.
Fixes: c2cad53ffc22a ("meters: do not set a defaut meter size from userspace")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index c4a09797..df327e95 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2687,7 +2687,7 @@ static struct stmt *json_parse_meter_stmt(struct json_ctx *ctx,
json_t *jkey, *jstmt;
struct stmt *stmt;
const char *name;
- uint32_t size = 0xffff;
+ uint32_t size = 0;
if (json_unpack_err(ctx, value, "{s:s, s:o, s:o}",
"name", &name, "key", &jkey, "stmt", &jstmt))
--
2.33.0

View File

@ -0,0 +1,30 @@
From 407e947dfc53827bd27689f2de9ed7f14f1b092e Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 20 Sep 2023 19:33:40 +0200
Subject: [PATCH] parser_json: Fix flowtable prio value parsing
Using format specifier 'I' requires a 64bit variable to write into. The
temporary variable 'prio' is of type int, though.
Fixes: 586ad210368b7 ("libnftables: Implement JSON parser")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index b41ddf2e..57c2c2a9 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3374,7 +3374,7 @@ static struct cmd *json_parse_cmd_add_flowtable(struct json_ctx *ctx,
if (op == CMD_DELETE || op == CMD_LIST || op == CMD_DESTROY)
return cmd_alloc(op, cmd_obj, &h, int_loc, NULL);
- if (json_unpack_err(ctx, root, "{s:s, s:I}",
+ if (json_unpack_err(ctx, root, "{s:s, s:i}",
"hook", &hook,
"prio", &prio)) {
handle_free(&h);
--
2.33.0

View File

@ -0,0 +1,45 @@
From d73e269f7bffc04b1163ffd16e0bc1689d4127d2 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 20 Sep 2023 19:40:11 +0200
Subject: [PATCH] parser_json: Fix synproxy object mss/wscale parsing
The fields are 16 and 8 bits in size, introduce temporary variables to
parse into.
Fixes: f44ab88b1088e ("src: add synproxy stateful object support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index ddd9c496..6d8e5c62 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3447,7 +3447,7 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
{
const char *family, *tmp, *rate_unit = "packets", *burst_unit = "bytes";
uint32_t l3proto = NFPROTO_UNSPEC;
- int inv = 0, flags = 0, i;
+ int inv = 0, flags = 0, i, j;
struct handle h = { 0 };
struct obj *obj;
json_t *jflags;
@@ -3634,11 +3634,12 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
case CMD_OBJ_SYNPROXY:
obj->type = NFT_OBJECT_SYNPROXY;
if (json_unpack_err(ctx, root, "{s:i, s:i}",
- "mss", &obj->synproxy.mss,
- "wscale", &obj->synproxy.wscale)) {
+ "mss", &i, "wscale", &j)) {
obj_free(obj);
return NULL;
}
+ obj->synproxy.mss = i;
+ obj->synproxy.wscale = j;
obj->synproxy.flags |= NF_SYNPROXY_OPT_MSS;
obj->synproxy.flags |= NF_SYNPROXY_OPT_WSCALE;
if (!json_unpack(root, "{s:o}", "flags", &jflags)) {
--
2.33.0

View File

@ -0,0 +1,30 @@
From 300edcfbb357ef1785b5a16424952fcd06146cb3 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 13 Sep 2023 20:44:19 +0200
Subject: [PATCH] parser_json: Fix typo in json_parse_cmd_add_object()
A case of bad c'n'p in the fixed commit broke ct timeout objects
parsing.
Fixes: c7a5401943df8 ("parser_json: Fix for ineffective family value checks")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index 9532f7be..da056814 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3570,7 +3570,7 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
obj_free(obj);
return NULL;
}
- obj->ct_helper.l3proto = l3proto;
+ obj->ct_timeout.l3proto = l3proto;
init_list_head(&obj->ct_timeout.timeout_list);
if (json_parse_ct_timeout_policy(ctx, root, obj)) {
--
2.33.0

View File

@ -0,0 +1,50 @@
From 34c1337296807b3a3147c95268f5e4ca70811779 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 20 Sep 2023 19:11:45 +0200
Subject: [PATCH] parser_json: Proper ct expectation attribute parsing
Parts of the code were unsafe (parsing 'I' format into uint32_t), the
rest just plain wrong (parsing 'o' format into char *tmp). Introduce a
temporary int variable to parse into.
Fixes: 1dd08fcfa07a4 ("src: add ct expectations support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index da056814..b41ddf2e 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3447,8 +3447,8 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
{
const char *family, *tmp, *rate_unit = "packets", *burst_unit = "bytes";
uint32_t l3proto = NFPROTO_UNSPEC;
+ int inv = 0, flags = 0, i;
struct handle h = { 0 };
- int inv = 0, flags = 0;
struct obj *obj;
json_t *jflags;
@@ -3599,11 +3599,12 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
return NULL;
}
}
- if (!json_unpack(root, "{s:o}", "dport", &tmp))
- obj->ct_expect.dport = atoi(tmp);
- json_unpack(root, "{s:I}", "timeout", &obj->ct_expect.timeout);
- if (!json_unpack(root, "{s:o}", "size", &tmp))
- obj->ct_expect.size = atoi(tmp);
+ if (!json_unpack(root, "{s:i}", "dport", &i))
+ obj->ct_expect.dport = i;
+ if (!json_unpack(root, "{s:i}", "timeout", &i))
+ obj->ct_expect.timeout = i;
+ if (!json_unpack(root, "{s:i}", "size", &i))
+ obj->ct_expect.size = i;
break;
case CMD_OBJ_LIMIT:
obj->type = NFT_OBJECT_LIMIT;
--
2.33.0

View File

@ -0,0 +1,31 @@
From 1e5ad2eeb38af0af2e06d4cba0ec4d84009855fa Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 13 Sep 2023 20:53:41 +0200
Subject: [PATCH] parser_json: Wrong check in json_parse_ct_timeout_policy()
The conditional around json_unpack() was meant to accept a missing
policy attribute. But the accidentally inverted check made the function
either ignore a given policy or access uninitialized memory.
Fixes: c82a26ebf7e9f ("json: Add ct timeout support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/parser_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parser_json.c b/src/parser_json.c
index 6d8e5c62..eec73034 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3415,7 +3415,7 @@ static int json_parse_ct_timeout_policy(struct json_ctx *ctx,
json_t *tmp, *val;
const char *key;
- if (!json_unpack(root, "{s:o}", "policy", &tmp))
+ if (json_unpack(root, "{s:o}", "policy", &tmp))
return 0;
if (!json_is_object(tmp)) {
--
2.33.0

View File

@ -0,0 +1,66 @@
From 33706f99ce56e178b058b180661aacbea2e79ce9 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 18 Aug 2023 11:40:39 +0200
Subject: [PATCH] py: fix exception during cleanup of half-initialized Nftables
When we create a Nftables instance against an older library version,
we might not find a symbol and fail with an exception when initializing
the context object.
Then, __del__() is still called, but resulting in a second exception
because self.__ctx is not set. Avoid that second exception.
$ python -c 'import nftables; nftables.Nftables()'
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/data/src/nftables/py/nftables.py", line 90, in __init__
self.nft_ctx_input_get_flags = lib.nft_ctx_input_get_flags
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/ctypes/__init__.py", line 389, in __getattr__
func = self.__getitem__(name)
^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.11/ctypes/__init__.py", line 394, in __getitem__
func = self._FuncPtr((name_or_ordinal, self))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: /lib64/libnftables.so.1: undefined symbol: nft_ctx_input_get_flags
Exception ignored in: <function Nftables.__del__ at 0x7f6315a2c540>
Traceback (most recent call last):
File "/data/src/nftables/py/nftables.py", line 166, in __del__
self.nft_ctx_free(self.__ctx)
^^^^^^^^^^^^^^^^^
AttributeError: 'Nftables' object has no attribute 'nft_ctx_free'
Signed-off-by: Thomas Haller <thaller@redhat.com>
Reviewed-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
py/nftables.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/py/nftables.py b/py/nftables.py
index 68fcd7dd..b1186781 100644
--- a/py/nftables.py
+++ b/py/nftables.py
@@ -74,6 +74,8 @@ class Nftables:
is requested from the library and buffering of output and error streams
is turned on.
"""
+ self.__ctx = None
+
lib = cdll.LoadLibrary(sofile)
### API function definitions
@@ -150,7 +152,9 @@ class Nftables:
self.nft_ctx_buffer_error(self.__ctx)
def __del__(self):
- self.nft_ctx_free(self.__ctx)
+ if self.__ctx is not None:
+ self.nft_ctx_free(self.__ctx)
+ self.__ctx = None
def __get_output_flag(self, name):
flag = self.output_flags[name]
--
2.33.0

View File

@ -0,0 +1,56 @@
From b73298405cda74b3a87a1818bb92f53298d34170 Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Mon, 15 Jan 2024 14:27:15 +0100
Subject: [PATCH] rule: fix sym refcount assertion
Scope release must happen last.
afl provided a reproducer where policy is a define, because
scope is released too early we get:
nft: src/rule.c:559: scope_release: Assertion `sym->refcnt == 1' failed.
... because chain->policy is EXPR_SYMBOL.
Fixes: 627c451b2351 ("src: allow variables in the chain priority specification")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/rule.c | 6 +++++-
tests/shell/testcases/bogons/nft-f/define_policy_assert | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)
create mode 100644 tests/shell/testcases/bogons/nft-f/define_policy_assert
diff --git a/src/rule.c b/src/rule.c
index 172ba1f6..342c43fb 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -729,7 +729,6 @@ void chain_free(struct chain *chain)
list_for_each_entry_safe(rule, next, &chain->rules, list)
rule_free(rule);
handle_free(&chain->handle);
- scope_release(&chain->scope);
xfree(chain->type.str);
expr_free(chain->dev_expr);
for (i = 0; i < chain->dev_array_len; i++)
@@ -738,6 +737,11 @@ void chain_free(struct chain *chain)
expr_free(chain->priority.expr);
expr_free(chain->policy);
xfree(chain->comment);
+
+ /* MUST be released after all expressions, they could
+ * hold refcounts.
+ */
+ scope_release(&chain->scope);
xfree(chain);
}
diff --git a/tests/shell/testcases/bogons/nft-f/define_policy_assert b/tests/shell/testcases/bogons/nft-f/define_policy_assert
new file mode 100644
index 00000000..f1e58b55
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/define_policy_assert
@@ -0,0 +1,3 @@
+chain y x { priority filter
+define p = foo
+policy $p
--
2.33.0

View File

@ -1,6 +1,6 @@
Name: nftables
Version: 1.0.8
Release: 2
Release: 3
Epoch: 1
Summary: A subsystem of the Linux kernel processing network data
License: GPLv2
@ -10,6 +10,44 @@ Source1: nftables.service
Source2: nftables.conf
Patch0001: create-standlone-module-dir.patch
Patch0002: backport-exthdr-prefer-raw_type-instead-of-desc-type.patch
Patch0003: backport-libnftables-Drop-cache-in-c-check-mode.patch
Patch0004: backport-py-fix-exception-during-cleanup-of-half-initialized-.patch
Patch0005: backport-evaluate-fix-check-for-truncation-in-stmt_evaluate_l.patch
Patch0006: backport-evaluate-do-not-remove-anonymous-set-with-protocol-f.patch
Patch0007: backport-evaluate-revisit-anonymous-set-with-single-element-o.patch
Patch0008: backport-evaluate-skip-anonymous-set-optimization-for-concate.patch
Patch0009: backport-datatype-fix-leak-and-cleanup-reference-counting-for.patch
Patch0010: backport-evaluate-fix-memleak-in-prefix-evaluation-with-wildc.patch
Patch0011: backport-netlink-fix-leaking-typeof_expr_data-typeof_expr_key.patch
Patch0012: backport-datatype-initialize-TYPE_CT_LABEL-slot-in-datatype-a.patch
Patch0013: backport-datatype-initialize-TYPE_CT_EVENTBIT-slot-in-datatyp.patch
Patch0014: backport-netlink-handle-invalid-etype-in-set_make_key.patch
Patch0015: backport-parser_json-Default-meter-size-to-zero.patch
Patch0016: backport-parser_json-Fix-flowtable-prio-value-parsing.patch
Patch0017: backport-parser_json-Proper-ct-expectation-attribute-parsing.patch
Patch0018: backport-parser_json-Fix-synproxy-object-mss-wscale-parsing.patch
Patch0019: backport-parser_json-Fix-typo-in-json_parse_cmd_add_object.patch
Patch0020: backport-parser_json-Wrong-check-in-json_parse_ct_timeout_pol.patch
Patch0021: backport-parser_json-Catch-nonsense-ops-in-match-statement.patch
Patch0022: backport-json-expose-dynamic-flag.patch
Patch0023: backport-evaluate-validate-maximum-log-statement-prefix-lengt.patch
Patch0024: backport-evaluate-reject-set-in-concatenation.patch
Patch0025: backport-datatype-don-t-return-a-const-string-from-cgroupv2_g.patch
Patch0026: backport-json-fix-use-after-free-in-table_flags_json.patch
Patch0027: backport-evaluate-fix-double-free-on-dtype-release.patch
Patch0028: backport-evaluate-validate-chain-max-length.patch
Patch0029: backport-parser_bison-fix-memleak-in-meta-set-error-handling.patch
Patch0030: backport-parser_bison-make-sure-obj_free-releases-timeout-pol.patch
Patch0031: backport-parser_bison-fix-ct-scope-underflow-if-ct-helper-sec.patch
Patch0032: backport-evaluate-stmt_nat-set-reference-must-point-to-a-map.patch
Patch0033: backport-meta-fix-tc-classid-parsing-out-of-bounds-access.patch
Patch0034: backport-netlink-don-t-crash-if-prefix-for-byte-is-requested.patch
Patch0035: backport-evaluate-don-t-crash-if-object-map-does-not-refer-to.patch
Patch0036: backport-evaluate-error-out-when-expression-has-no-datatype.patch
Patch0037: backport-evaluate-tproxy-move-range-error-checks-after-arg-ev.patch
Patch0038: backport-evaluate-error-out-when-store-needs-more-than-one-12.patch
Patch0039: backport-rule-fix-sym-refcount-assertion.patch
BuildRequires: gcc flex bison libmnl-devel gmp-devel readline-devel libnftnl-devel docbook2X systemd
BuildRequires: iptables-devel jansson-devel python3-devel
@ -109,6 +147,49 @@ echo "%{_libdir}" > %{buildroot}/etc/ld.so.conf.d/%{name}-%{_arch}.conf
%{python3_sitelib}/nftables/
%changelog
* Fri Apr 19 2024 lingsheng <lingsheng1@h-partners.com> - 1:1.0.8-3
- Type:bugfix
- CVE:NA
- SUG:NA
- DESC:datatype: don't return a const string from cgroupv2_get_path()
datatype: fix leak and cleanup reference counting for struct datatype
datatype: initialize TYPE_CT_EVENTBIT slot in datatype array
datatype: initialize TYPE_CT_LABEL slot in datatype array
evaluate: do not remove anonymous set with protocol flags and single element
evaluate: don't crash if object map does not refer to a value
evaluate: error out when expression has no datatype
evaluate: error out when store needs more than one 128bit register of align fixup
evaluate: fix check for truncation in stmt_evaluate_log_prefix()
evaluate: fix double free on dtype release
evaluate: fix memleak in prefix evaluation with wildcard interface name
evaluate: reject set in concatenation
evaluate: revisit anonymous set with single element optimization
evaluate: skip anonymous set optimization for concatenations
evaluate: stmt_nat: set reference must point to a map
evaluate: tproxy: move range error checks after arg evaluation
evaluate: validate chain max length
evaluate: validate maximum log statement prefix length
exthdr: prefer raw_type instead of desc->type
json: expose dynamic flag
json: fix use after free in table_flags_json()
libnftables: Drop cache in -c/--check mode
meta: fix tc classid parsing out-of-bounds access
netlink: don't crash if prefix for < byte is requested
netlink: fix leaking typeof_expr_data/typeof_expr_key in netlink_delinearize_set()
netlink: handle invalid etype in set_make_key()
parser_bison: fix ct scope underflow if ct helper section is duplicated
parser_bison: fix memleak in meta set error handling
parser_bison: make sure obj_free releases timeout policies
parser_json: Catch nonsense ops in match statement
parser_json: Default meter size to zero
parser_json: Fix flowtable prio value parsing
parser_json: Fix synproxy object mss/wscale parsing
parser_json: Fix typo in json_parse_cmd_add_object()
parser_json: Proper ct expectation attribute parsing
parser_json: Wrong check in json_parse_ct_timeout_policy()
py: fix exception during cleanup of half-initialized Nftables
rule: fix sym refcount assertion
* Wed Mar 13 2024 zhouyihang <zhouyihang3@h-partners.com> - 1:1.0.8-2
- Type: bugfix
- ID: NA