Do not inconditionally hold reference to start interval.
The handling depends on what kind of range expression we need to build,
either no range at all, a prefix or a plain range. Depending on the
case, we need to partially clone what we need from the expression to
avoid use-after-free.
This fixes valgrind reports that look like this, when listing rulesets:
==30018== 2,057,984 (1,028,992 direct, 1,028,992 indirect) bytes in 8,039 blocks are definitely lost in loss record 76 of 83
==30018== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==30018== by 0x4E75978: xmalloc (utils.c:36)
==30018== by 0x4E75A5D: xzalloc (utils.c:65)
==30018== by 0x4E5CEC0: expr_alloc (expression.c:45)
==30018== by 0x4E5D610: mapping_expr_alloc (expression.c:985)
==30018== by 0x4E6A068: netlink_delinearize_setelem (netlink.c:810)
==30018== by 0x5B51320: nftnl_set_elem_foreach (set_elem.c:673)
==30018== by 0x4E6A2D5: netlink_list_setelems (netlink.c:864)
==30018== by 0x4E56C76: cache_init_objects (rule.c:166)
==30018== by 0x4E56C76: cache_init (rule.c:216)
==30018== by 0x4E56C76: cache_update (rule.c:243)
==30018== by 0x4E64530: cmd_evaluate_list (evaluate.c:3503)
==30018== by 0x4E64530: cmd_evaluate (evaluate.c:3880)
==30018== by 0x4E7D12F: nft_parse (parser_bison.y:798)
==30018== by 0x4E7AB56: nft_parse_bison_buffer (libnftables.c:349)
==30018== by 0x4E7AB56: nft_run_cmd_from_buffer (libnftables.c:394)
Reported-by: Václav Zindulka <vaclav.zindulka@xxxxxxxxxx>
Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
---
src/segtree.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/segtree.c b/src/segtree.c
index 6ee655238b7d..4353e85a02c5 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -872,8 +872,7 @@ void interval_map_decompose(struct expr *set)
low = i;
continue;
}
- } else
- expr_get(low);
+ }
mpz_sub(range, expr_value(i)->value, expr_value(low)->value);
mpz_sub_ui(range, range, 1);
@@ -881,7 +880,7 @@ void interval_map_decompose(struct expr *set)
mpz_and(p, expr_value(low)->value, range);
if (!mpz_cmp_ui(range, 0))
- compound_expr_add(set, low);
+ compound_expr_add(set, expr_get(low));
else if ((!range_is_prefix(range) ||
!(i->dtype->flags & DTYPE_F_PREFIX)) ||
mpz_cmp_ui(p, 0)) {
@@ -894,7 +893,9 @@ void interval_map_decompose(struct expr *set)
mpz_add(range, range, expr_value(low)->value);
mpz_set(tmp->value, range);
- tmp = range_expr_alloc(&low->location, expr_value(low), tmp);
+ tmp = range_expr_alloc(&low->location,
+ expr_clone(expr_value(low)),
+ tmp);
tmp = set_elem_expr_alloc(&low->location, tmp);
if (low->etype == EXPR_MAPPING) {
@@ -906,7 +907,7 @@ void interval_map_decompose(struct expr *set)
tmp->expiration = low->left->expiration;
tmp = mapping_expr_alloc(&tmp->location, tmp,
- low->right);
+ expr_clone(low->right));
} else {
if (low->comment)
tmp->comment = xstrdup(low->comment);
@@ -922,7 +923,8 @@ void interval_map_decompose(struct expr *set)
unsigned int prefix_len;
prefix_len = expr_value(i)->len - mpz_scan0(range, 0);
- prefix = prefix_expr_alloc(&low->location, expr_value(low),
+ prefix = prefix_expr_alloc(&low->location,
+ expr_clone(expr_value(low)),
prefix_len);
prefix->len = expr_value(i)->len;
@@ -937,7 +939,7 @@ void interval_map_decompose(struct expr *set)
prefix->expiration = low->left->expiration;
prefix = mapping_expr_alloc(&low->location, prefix,
- low->right);
+ expr_clone(low->right));
} else {
if (low->comment)
prefix->comment = xstrdup(low->comment);
--
2.11.0
