diff --git a/backport-datatype-don-t-return-a-const-string-from-cgroupv2_g.patch b/backport-datatype-don-t-return-a-const-string-from-cgroupv2_g.patch new file mode 100644 index 0000000000000000000000000000000000000000..b807d538f80987716f10223e46ed4a04ef2953ff --- /dev/null +++ b/backport-datatype-don-t-return-a-const-string-from-cgroupv2_g.patch @@ -0,0 +1,44 @@ +From 1f867d0d07122f54f76e20af3c636ce66102b683 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-datatype-fix-leak-and-cleanup-reference-counting-for.patch b/backport-datatype-fix-leak-and-cleanup-reference-counting-for.patch new file mode 100644 index 0000000000000000000000000000000000000000..9d3f4492db5a668c3726fc1112eec78e6cf1883f --- /dev/null +++ b/backport-datatype-fix-leak-and-cleanup-reference-counting-for.patch @@ -0,0 +1,485 @@ +From 8519ab031d8022999603a69ee9f18e8cfb06645d Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-datatype-initialize-TYPE_CT_EVENTBIT-slot-in-datatyp.patch b/backport-datatype-initialize-TYPE_CT_EVENTBIT-slot-in-datatyp.patch new file mode 100644 index 0000000000000000000000000000000000000000..8fa0bcc95d08aa7d44fa05901861e32ec08cae6a --- /dev/null +++ b/backport-datatype-initialize-TYPE_CT_EVENTBIT-slot-in-datatyp.patch @@ -0,0 +1,59 @@ +From 2d9e23a9e9c3c729784a3add41639cbd3f72d504 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-datatype-initialize-TYPE_CT_LABEL-slot-in-datatype-a.patch b/backport-datatype-initialize-TYPE_CT_LABEL-slot-in-datatype-a.patch new file mode 100644 index 0000000000000000000000000000000000000000..4e351ddf375dfe7d12c7a9514387a6a9b03bea94 --- /dev/null +++ b/backport-datatype-initialize-TYPE_CT_LABEL-slot-in-datatype-a.patch @@ -0,0 +1,71 @@ +From 1b235f9962a059a599d9a9ecce477ed71e328e89 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-evaluate-do-not-remove-anonymous-set-with-protocol-f.patch b/backport-evaluate-do-not-remove-anonymous-set-with-protocol-f.patch new file mode 100644 index 0000000000000000000000000000000000000000..12c2a3cffb6bea740306005c89971dae5d674d97 --- /dev/null +++ b/backport-evaluate-do-not-remove-anonymous-set-with-protocol-f.patch @@ -0,0 +1,53 @@ +From 01167c393a12c74c6f9a3015b75702964ff5bcda Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-evaluate-don-t-crash-if-object-map-does-not-refer-to.patch b/backport-evaluate-don-t-crash-if-object-map-does-not-refer-to.patch new file mode 100644 index 0000000000000000000000000000000000000000..7289d47c1349f2e6e1fc5db6846197b916b85480 --- /dev/null +++ b/backport-evaluate-don-t-crash-if-object-map-does-not-refer-to.patch @@ -0,0 +1,50 @@ +From 588470e00539404fd793fe22718067721f5754be Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-error-out-when-expression-has-no-datatype.patch b/backport-evaluate-error-out-when-expression-has-no-datatype.patch new file mode 100644 index 0000000000000000000000000000000000000000..ebee3ff4a4e867df9459a1ed05c0c58cc4ce3a5f --- /dev/null +++ b/backport-evaluate-error-out-when-expression-has-no-datatype.patch @@ -0,0 +1,43 @@ +From 666018e71ebb5df376b1b013c1ca859eaed66f1a Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-error-out-when-store-needs-more-than-one-12.patch b/backport-evaluate-error-out-when-store-needs-more-than-one-12.patch new file mode 100644 index 0000000000000000000000000000000000000000..65dbd34d5e8ba150034cee37471cc350c1c7bfa6 --- /dev/null +++ b/backport-evaluate-error-out-when-store-needs-more-than-one-12.patch @@ -0,0 +1,54 @@ +From 8a66de2a15943b2fbf960967cdbcbd0a148cb114 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-fix-check-for-truncation-in-stmt_evaluate_l.patch b/backport-evaluate-fix-check-for-truncation-in-stmt_evaluate_l.patch new file mode 100644 index 0000000000000000000000000000000000000000..ec5fe1ec1d936102d4121e72b764544bcdea9971 --- /dev/null +++ b/backport-evaluate-fix-check-for-truncation-in-stmt_evaluate_l.patch @@ -0,0 +1,42 @@ +From 7e6aa6db1fe5b14b5d224da11b077c50cc954efa Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-evaluate-fix-double-free-on-dtype-release.patch b/backport-evaluate-fix-double-free-on-dtype-release.patch new file mode 100644 index 0000000000000000000000000000000000000000..2e3f735f0a0877c43c482f4e50c68627a55f839c --- /dev/null +++ b/backport-evaluate-fix-double-free-on-dtype-release.patch @@ -0,0 +1,43 @@ +From ee73e9dcd46dc5a1fe3be7caa8b9323819e394b8 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-fix-memleak-in-prefix-evaluation-with-wildc.patch b/backport-evaluate-fix-memleak-in-prefix-evaluation-with-wildc.patch new file mode 100644 index 0000000000000000000000000000000000000000..095b1439b00f119a4ccce84f155328fca6d6d892 --- /dev/null +++ b/backport-evaluate-fix-memleak-in-prefix-evaluation-with-wildc.patch @@ -0,0 +1,60 @@ +From fe727d5da18c40cb9f002eeaf0116f59e9600659 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-evaluate-reject-set-in-concatenation.patch b/backport-evaluate-reject-set-in-concatenation.patch new file mode 100644 index 0000000000000000000000000000000000000000..714ad8f3269339d2ef80e2216b867c09422f02a5 --- /dev/null +++ b/backport-evaluate-reject-set-in-concatenation.patch @@ -0,0 +1,52 @@ +From 4b6a4ad9134fa71277c2ff7f92776e1faeb83000 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-evaluate-revisit-anonymous-set-with-single-element-o.patch b/backport-evaluate-revisit-anonymous-set-with-single-element-o.patch new file mode 100644 index 0000000000000000000000000000000000000000..5ab28000bfa88d378a772793827b8e5afb3eddcb --- /dev/null +++ b/backport-evaluate-revisit-anonymous-set-with-single-element-o.patch @@ -0,0 +1,116 @@ +From fa17b17ea74a21a44596f3212466ff3d2d3ede8e Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-evaluate-skip-anonymous-set-optimization-for-concate.patch b/backport-evaluate-skip-anonymous-set-optimization-for-concate.patch new file mode 100644 index 0000000000000000000000000000000000000000..d8cd1d607edf481ee608baac313ddd7f1da958a7 --- /dev/null +++ b/backport-evaluate-skip-anonymous-set-optimization-for-concate.patch @@ -0,0 +1,49 @@ +From 6bc6673fc88c8a3e3dd5504b2d24a6d6bc2f8427 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-evaluate-stmt_nat-set-reference-must-point-to-a-map.patch b/backport-evaluate-stmt_nat-set-reference-must-point-to-a-map.patch new file mode 100644 index 0000000000000000000000000000000000000000..b3eb2d992a77f93825dac4aeea7ec1053bdffb8f --- /dev/null +++ b/backport-evaluate-stmt_nat-set-reference-must-point-to-a-map.patch @@ -0,0 +1,64 @@ +From 3eb0a73a9ee32897290d4097c0ec29377e25859e Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-tproxy-move-range-error-checks-after-arg-ev.patch b/backport-evaluate-tproxy-move-range-error-checks-after-arg-ev.patch new file mode 100644 index 0000000000000000000000000000000000000000..fdfbb08c60e8320280395677d60fe5a3ed278536 --- /dev/null +++ b/backport-evaluate-tproxy-move-range-error-checks-after-arg-ev.patch @@ -0,0 +1,69 @@ +From 1d03ab5267bdbc7c0bcb041efaad42a462fdeb5f Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-validate-chain-max-length.patch b/backport-evaluate-validate-chain-max-length.patch new file mode 100644 index 0000000000000000000000000000000000000000..23ac4a923020eec147e0ce7eee478a387d902974 --- /dev/null +++ b/backport-evaluate-validate-chain-max-length.patch @@ -0,0 +1,105 @@ +From 08925ba0daf19753df933fed69f4572a7c9d3d47 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-evaluate-validate-maximum-log-statement-prefix-lengt.patch b/backport-evaluate-validate-maximum-log-statement-prefix-lengt.patch new file mode 100644 index 0000000000000000000000000000000000000000..eb9523d4a20c8039690b8ff265015e770dae5ff9 --- /dev/null +++ b/backport-evaluate-validate-maximum-log-statement-prefix-lengt.patch @@ -0,0 +1,36 @@ +From 6ceec21204e0260af2d50e9e987d0fe3c79c28d4 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-exthdr-prefer-raw_type-instead-of-desc-type.patch b/backport-exthdr-prefer-raw_type-instead-of-desc-type.patch new file mode 100644 index 0000000000000000000000000000000000000000..ddddeb8d9fb1d5db785945dcfb3a2522c46fab94 --- /dev/null +++ b/backport-exthdr-prefer-raw_type-instead-of-desc-type.patch @@ -0,0 +1,40 @@ +From 574fa55c2e70449887c7714d7b043f4e8b6d28da Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-json-expose-dynamic-flag.patch b/backport-json-expose-dynamic-flag.patch new file mode 100644 index 0000000000000000000000000000000000000000..1f901f396ceeb9e045315bc8585af2e2c11c5e68 --- /dev/null +++ b/backport-json-expose-dynamic-flag.patch @@ -0,0 +1,45 @@ +From 57f5aca0006ebf984ffc1f66d48cf3b74a3d1f59 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-json-fix-use-after-free-in-table_flags_json.patch b/backport-json-fix-use-after-free-in-table_flags_json.patch new file mode 100644 index 0000000000000000000000000000000000000000..9122fe2e97302c78c077424ee3df0949a398dd38 --- /dev/null +++ b/backport-json-fix-use-after-free-in-table_flags_json.patch @@ -0,0 +1,49 @@ +From b04512cf30de1ba6657facba5ebe2321e17c2727 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-libnftables-Drop-cache-in-c-check-mode.patch b/backport-libnftables-Drop-cache-in-c-check-mode.patch new file mode 100644 index 0000000000000000000000000000000000000000..65290e9c15ad7336ceebd68748fde9455170c762 --- /dev/null +++ b/backport-libnftables-Drop-cache-in-c-check-mode.patch @@ -0,0 +1,100 @@ +From 458e91a954abe4b7fb4ba70901c7da28220c446a Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +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 +--- + 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 + diff --git a/backport-meta-fix-tc-classid-parsing-out-of-bounds-access.patch b/backport-meta-fix-tc-classid-parsing-out-of-bounds-access.patch new file mode 100644 index 0000000000000000000000000000000000000000..218dafa23f38f9e9fe000cb612285cc00a3a2442 --- /dev/null +++ b/backport-meta-fix-tc-classid-parsing-out-of-bounds-access.patch @@ -0,0 +1,104 @@ +From 7008b1200fb4988b7cd7ee1c5399cae071688d50 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-netlink-don-t-crash-if-prefix-for-byte-is-requested.patch b/backport-netlink-don-t-crash-if-prefix-for-byte-is-requested.patch new file mode 100644 index 0000000000000000000000000000000000000000..7d25c4f81ebd061b960300821ca01a00aa7a0bdb --- /dev/null +++ b/backport-netlink-don-t-crash-if-prefix-for-byte-is-requested.patch @@ -0,0 +1,148 @@ +From 0404ff08b3c18052e6689d75fa85275d3cef7e8e Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-netlink-fix-leaking-typeof_expr_data-typeof_expr_key.patch b/backport-netlink-fix-leaking-typeof_expr_data-typeof_expr_key.patch new file mode 100644 index 0000000000000000000000000000000000000000..f08a011cbb4155f553b0c26a5969cffb6bb05e78 --- /dev/null +++ b/backport-netlink-fix-leaking-typeof_expr_data-typeof_expr_key.patch @@ -0,0 +1,79 @@ +From a76c8960905ef6803c0c79df7bc0a030209c9455 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-netlink-handle-invalid-etype-in-set_make_key.patch b/backport-netlink-handle-invalid-etype-in-set_make_key.patch new file mode 100644 index 0000000000000000000000000000000000000000..779e0f25da0aa4dfe83216a6914f8618d723e8f4 --- /dev/null +++ b/backport-netlink-handle-invalid-etype-in-set_make_key.patch @@ -0,0 +1,31 @@ +From c4186c5376ee73efff005dbd23dd73a8e06e6ad8 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-parser_bison-fix-ct-scope-underflow-if-ct-helper-sec.patch b/backport-parser_bison-fix-ct-scope-underflow-if-ct-helper-sec.patch new file mode 100644 index 0000000000000000000000000000000000000000..0cfecf8425fe13efece0c24a01ab7e0dfb12e976 --- /dev/null +++ b/backport-parser_bison-fix-ct-scope-underflow-if-ct-helper-sec.patch @@ -0,0 +1,109 @@ +From 037d58a27d675802286aafb23e409b8c1d3eef56 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 */ { $$ = $-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 */ { $$ = $
-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 */ { $$ = $
-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 = &$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 + diff --git a/backport-parser_bison-fix-memleak-in-meta-set-error-handling.patch b/backport-parser_bison-fix-memleak-in-meta-set-error-handling.patch new file mode 100644 index 0000000000000000000000000000000000000000..e13550c8835519222ce35e9ee0b35da20a468c76 --- /dev/null +++ b/backport-parser_bison-fix-memleak-in-meta-set-error-handling.patch @@ -0,0 +1,41 @@ +From 21608263cc1ae489326e743957bfe34b05414a44 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/backport-parser_bison-make-sure-obj_free-releases-timeout-pol.patch b/backport-parser_bison-make-sure-obj_free-releases-timeout-pol.patch new file mode 100644 index 0000000000000000000000000000000000000000..d86f4e355f569a0fe444bd4d0cebea6566a42205 --- /dev/null +++ b/backport-parser_bison-make-sure-obj_free-releases-timeout-pol.patch @@ -0,0 +1,43 @@ +From d5a06af393eaf47571c884a265d1f6e6ba34ed97 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 */ + { + $$ = $-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 + diff --git a/backport-parser_json-Catch-nonsense-ops-in-match-statement.patch b/backport-parser_json-Catch-nonsense-ops-in-match-statement.patch new file mode 100644 index 0000000000000000000000000000000000000000..449959464e3add52bd2e446d7336ebdc25f1f3d7 --- /dev/null +++ b/backport-parser_json-Catch-nonsense-ops-in-match-statement.patch @@ -0,0 +1,44 @@ +From 7df0b2f1a1c64e2bdc652fd2418b4f7218c93f1f Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Default-meter-size-to-zero.patch b/backport-parser_json-Default-meter-size-to-zero.patch new file mode 100644 index 0000000000000000000000000000000000000000..f8cbf89993cfbee3c5ee69c5bacb9bb6b54c9758 --- /dev/null +++ b/backport-parser_json-Default-meter-size-to-zero.patch @@ -0,0 +1,30 @@ +From 343006f5e0a6cdf907877218a354505dcc9d9864 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Fix-flowtable-prio-value-parsing.patch b/backport-parser_json-Fix-flowtable-prio-value-parsing.patch new file mode 100644 index 0000000000000000000000000000000000000000..7bbbfd57da9326b5ca6f636757d52c44b2a1ca0c --- /dev/null +++ b/backport-parser_json-Fix-flowtable-prio-value-parsing.patch @@ -0,0 +1,30 @@ +From 407e947dfc53827bd27689f2de9ed7f14f1b092e Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Fix-synproxy-object-mss-wscale-parsing.patch b/backport-parser_json-Fix-synproxy-object-mss-wscale-parsing.patch new file mode 100644 index 0000000000000000000000000000000000000000..f70f543f6378c3ec6dabbe9f45b1113252ad868e --- /dev/null +++ b/backport-parser_json-Fix-synproxy-object-mss-wscale-parsing.patch @@ -0,0 +1,45 @@ +From d73e269f7bffc04b1163ffd16e0bc1689d4127d2 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Fix-typo-in-json_parse_cmd_add_object.patch b/backport-parser_json-Fix-typo-in-json_parse_cmd_add_object.patch new file mode 100644 index 0000000000000000000000000000000000000000..45cd92e91b2da72924b36cf8268e36b064e805b6 --- /dev/null +++ b/backport-parser_json-Fix-typo-in-json_parse_cmd_add_object.patch @@ -0,0 +1,30 @@ +From 300edcfbb357ef1785b5a16424952fcd06146cb3 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Proper-ct-expectation-attribute-parsing.patch b/backport-parser_json-Proper-ct-expectation-attribute-parsing.patch new file mode 100644 index 0000000000000000000000000000000000000000..0a908a16cdae1afddabe0b0cdca1056b914970d7 --- /dev/null +++ b/backport-parser_json-Proper-ct-expectation-attribute-parsing.patch @@ -0,0 +1,50 @@ +From 34c1337296807b3a3147c95268f5e4ca70811779 Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-parser_json-Wrong-check-in-json_parse_ct_timeout_pol.patch b/backport-parser_json-Wrong-check-in-json_parse_ct_timeout_pol.patch new file mode 100644 index 0000000000000000000000000000000000000000..d83cb42ecbf212b31131194ed05fbdabe0c643be --- /dev/null +++ b/backport-parser_json-Wrong-check-in-json_parse_ct_timeout_pol.patch @@ -0,0 +1,31 @@ +From 1e5ad2eeb38af0af2e06d4cba0ec4d84009855fa Mon Sep 17 00:00:00 2001 +From: Phil Sutter +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 +--- + 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 + diff --git a/backport-py-fix-exception-during-cleanup-of-half-initialized-.patch b/backport-py-fix-exception-during-cleanup-of-half-initialized-.patch new file mode 100644 index 0000000000000000000000000000000000000000..b465040e1ca582bd6a459659158c7d82bc0925bd --- /dev/null +++ b/backport-py-fix-exception-during-cleanup-of-half-initialized-.patch @@ -0,0 +1,66 @@ +From 33706f99ce56e178b058b180661aacbea2e79ce9 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +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 "", line 1, in + 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: + 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 +Reviewed-by: Phil Sutter +Signed-off-by: Pablo Neira Ayuso +--- + 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 + diff --git a/backport-rule-fix-sym-refcount-assertion.patch b/backport-rule-fix-sym-refcount-assertion.patch new file mode 100644 index 0000000000000000000000000000000000000000..418249e53a67745cef78c69d25acca5daa535065 --- /dev/null +++ b/backport-rule-fix-sym-refcount-assertion.patch @@ -0,0 +1,56 @@ +From b73298405cda74b3a87a1818bb92f53298d34170 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +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 +--- + 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 + diff --git a/nftables.spec b/nftables.spec index 1787a61d1b8f9f06a569129c826f3b54d8903f31..258ece1233c96e8512f49a64ed47135694760403 100644 --- a/nftables.spec +++ b/nftables.spec @@ -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 - 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 - 1:1.0.8-2 - Type: bugfix - ID: NA