Concatenations need a dynamic datatype. Set references use the datatype
from the set. Add a reference counter to deal with releasing the
dynamic datatype object when it is shared, which occurs from the
following paths:
* netlink_delinearize_setelem(), when elements refer to the datatype of
an existing set in the cache.
* set_expr_alloc(), for implicit set/maps and update statement from the
packet path.
* set_ref_expr_alloc(), for set and map references via @.
* expr_evaluate_set_elem(), when elements are added and they refer to
the set datatype, ie. add element command.
==28352== 1,350 (440 direct, 910 indirect) bytes in 5 blocks are definitely lost in loss recor 3 of 3
==28352== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==28352== by 0x4E79558: xmalloc (utils.c:36)
==28352== by 0x4E7963D: xzalloc (utils.c:65)
==28352== by 0x4E6029B: dtype_alloc (datatype.c:1073)
==28352== by 0x4E6029B: concat_type_alloc (datatype.c:1127)
==28352== by 0x4E6D3B3: netlink_delinearize_set (netlink.c:578)
==28352== by 0x4E6D68E: list_set_cb (netlink.c:648)
==28352== by 0x5D74023: nftnl_set_list_foreach (set.c:780)
==28352== by 0x4E6D6F3: netlink_list_sets (netlink.c:669)
==28352== by 0x4E5A7A3: cache_init_objects (rule.c:159)
==28352== by 0x4E5A7A3: cache_init (rule.c:216)
==28352== by 0x4E5A7A3: cache_update (rule.c:266)
==28352== by 0x4E7E0EE: nft_evaluate (libnftables.c:388)
==28352== by 0x4E7EADD: nft_run_cmd_from_filename (libnftables.c:479)
==28352== by 0x109A53: main (main.c:310)
Remove DTYPE_F_CLONE flag, which is replaced by proper reference counter.
Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
---
v2: deal with expr_evaluate_set_elem() path too.
include/datatype.h | 6 ++++--
src/datatype.c | 38 ++++++++++++++++++++++++++++----------
src/evaluate.c | 4 ++--
src/expression.c | 11 ++++++-----
src/netlink.c | 2 +-
5 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/include/datatype.h b/include/datatype.h
index 14ece282902c..5765c1ba7502 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -117,12 +117,10 @@ struct expr;
*
* @DTYPE_F_ALLOC: datatype is dynamically allocated
* @DTYPE_F_PREFIX: preferred representation for ranges is a prefix
- * @DTYPE_F_CLONE: this is an instance from original datatype
*/
enum datatype_flags {
DTYPE_F_ALLOC = (1 << 0),
DTYPE_F_PREFIX = (1 << 1),
- DTYPE_F_CLONE = (1 << 2),
};
/**
@@ -140,6 +138,7 @@ enum datatype_flags {
* @print: function to print a constant of this type
* @parse: function to parse a symbol and return an expression
* @sym_tbl: symbol table for this type
+ * @refcnt: reference counter (only for DTYPE_F_ALLOC)
*/
struct datatype {
uint32_t type;
@@ -158,10 +157,13 @@ struct datatype {
struct error_record *(*parse)(const struct expr *sym,
struct expr **res);
const struct symbol_table *sym_tbl;
+ unsigned int refcnt;
};
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_free(const struct datatype *dtype);
extern struct error_record *symbol_parse(const struct expr *sym,
struct expr **res);
diff --git a/src/datatype.c b/src/datatype.c
index 1d5ed6f798de..94abc91d8112 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1072,10 +1072,22 @@ static struct datatype *dtype_alloc(void)
dtype = xzalloc(sizeof(*dtype));
dtype->flags = DTYPE_F_ALLOC;
+ dtype->refcnt = 1;
return dtype;
}
+struct datatype *datatype_get(const struct datatype *ptr)
+{
+ struct datatype *dtype = (struct datatype *)ptr;
+
+ if (!(dtype->flags & DTYPE_F_ALLOC))
+ return dtype;
+
+ dtype->refcnt++;
+ return dtype;
+}
+
static struct datatype *dtype_clone(const struct datatype *orig_dtype)
{
struct datatype *dtype;
@@ -1084,18 +1096,24 @@ static struct datatype *dtype_clone(const struct datatype *orig_dtype)
*dtype = *orig_dtype;
dtype->name = xstrdup(orig_dtype->name);
dtype->desc = xstrdup(orig_dtype->desc);
- dtype->flags = DTYPE_F_ALLOC | DTYPE_F_CLONE;
+ dtype->flags = DTYPE_F_ALLOC;
+ dtype->refcnt = 1;
return dtype;
}
-static void dtype_free(const struct datatype *dtype)
+void datatype_free(const struct datatype *ptr)
{
- if (dtype->flags & DTYPE_F_ALLOC) {
- xfree(dtype->name);
- xfree(dtype->desc);
- xfree(dtype);
- }
+ struct datatype *dtype = (struct datatype *)ptr;
+
+ if (!(dtype->flags & DTYPE_F_ALLOC))
+ return;
+ if (--dtype->refcnt > 0)
+ return;
+
+ xfree(dtype->name);
+ xfree(dtype->desc);
+ xfree(dtype);
}
const struct datatype *concat_type_alloc(uint32_t type)
@@ -1137,7 +1155,7 @@ const struct datatype *concat_type_alloc(uint32_t type)
void concat_type_destroy(const struct datatype *dtype)
{
- dtype_free(dtype);
+ datatype_free(dtype);
}
const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype,
@@ -1157,8 +1175,8 @@ const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype,
void set_datatype_destroy(const struct datatype *dtype)
{
- if (dtype && dtype->flags & DTYPE_F_CLONE)
- dtype_free(dtype);
+ if (dtype)
+ datatype_free(dtype);
}
static struct error_record *time_unit_parse(const struct location *loc,
diff --git a/src/evaluate.c b/src/evaluate.c
index 39101b486b2f..6018c96edefe 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1239,7 +1239,7 @@ static int expr_evaluate_set_elem(struct eval_ctx *ctx, struct expr **expr)
}
}
- elem->dtype = elem->key->dtype;
+ elem->dtype = datatype_get(elem->key->dtype);
elem->len = elem->key->len;
elem->flags = elem->key->flags;
return 0;
@@ -1285,7 +1285,7 @@ static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
set->set_flags |= NFT_SET_CONSTANT;
- set->dtype = ctx->ectx.dtype;
+ set->dtype = datatype_get(ctx->ectx.dtype);
set->len = ctx->ectx.len;
set->flags |= EXPR_F_CONSTANT;
return 0;
diff --git a/src/expression.c b/src/expression.c
index ef694f2a194d..d1cfb24681d9 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -57,8 +57,8 @@ struct expr *expr_clone(const struct expr *expr)
{
struct expr *new;
- new = expr_alloc(&expr->location, expr->etype, expr->dtype,
- expr->byteorder, expr->len);
+ new = expr_alloc(&expr->location, expr->etype,
+ datatype_get(expr->dtype), expr->byteorder, expr->len);
new->flags = expr->flags;
new->op = expr->op;
expr_ops(expr)->clone(new, expr);
@@ -86,6 +86,8 @@ void expr_free(struct expr *expr)
if (--expr->refcnt > 0)
return;
+ datatype_free(expr->dtype);
+
/* EXPR_INVALID expressions lack ->ops structure.
* This happens for compound types.
*/
@@ -803,7 +805,6 @@ void compound_expr_remove(struct expr *compound, struct expr *expr)
static void concat_expr_destroy(struct expr *expr)
{
- concat_type_destroy(expr->dtype);
compound_expr_destroy(expr);
}
@@ -936,7 +937,7 @@ struct expr *set_expr_alloc(const struct location *loc, const struct set *set)
return set_expr;
set_expr->set_flags = set->flags;
- set_expr->dtype = set->key->dtype;
+ set_expr->dtype = datatype_get(set->key->dtype);
return set_expr;
}
@@ -1067,7 +1068,7 @@ struct expr *set_ref_expr_alloc(const struct location *loc, struct set *set)
{
struct expr *expr;
- expr = expr_alloc(loc, EXPR_SET_REF, set->key->dtype, 0, 0);
+ expr = expr_alloc(loc, EXPR_SET_REF, datatype_get(set->key->dtype), 0, 0);
expr->set = set_get(set);
expr->flags |= EXPR_F_CONSTANT;
return expr;
diff --git a/src/netlink.c b/src/netlink.c
index 7a4312498ce8..c5567c0d29ca 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -767,7 +767,7 @@ int netlink_delinearize_setelem(struct nftnl_set_elem *nlse,
flags = nftnl_set_elem_get_u32(nlse, NFTNL_SET_ELEM_FLAGS);
key = netlink_alloc_value(&netlink_location, &nld);
- key->dtype = set->key->dtype;
+ key->dtype = datatype_get(set->key->dtype);
key->byteorder = set->key->byteorder;
if (set->key->dtype->subtypes)
key = netlink_parse_concat_elem(set->key->dtype, key);
--
2.11.0
