src: add reference counter for dynamic datatypes

There are two datatypes are using runtime datatype allocation:

* Concatenations.
* Integer, that require byteorder adjustment.

From the evaluation / postprocess step, transformations are common,
hence expressions may end up fetching (infering) datatypes from an
existing one.

This patch adds a reference counter to release the dynamic datatype
object when it is shared.

The API includes the following helper functions:

* datatype_set(expr, datatype), to assign a datatype to an expression.
  This helper already deals with reference counting for dynamic
  datatypes. This also drops the reference counter of any previous
  datatype (to deal with the datatype replacement case).

* datatype_get(datatype) bumps the reference counter. This function also
  deals with nul-pointers, that occurs when the datatype is unset.

* datatype_free() drops the reference counter, and it also releases the
  datatype if there are not more clients of it.

Rule of thumb is: The reference counter of any newly allocated datatype
is set to zero.

This patch also updates every spot to use datatype_set() for non-dynamic
datatypes, for consistency. In this case, the helper just makes an
simple assignment.

Note that expr_alloc() has been updated to call datatype_get() on the
datatype that is assigned to this new expression. Moreover, expr_free()
calls datatype_free().

This fixes valgrind reports like this one:

==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)

This patch also removes the DTYPE_F_CLONE flag which is broken and not
needed anymore since proper reference counting is in place.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

1 files changed, 19 insertions, 18 deletions

diff --git a/src/evaluate.c b/src/evaluate.c
index 39101b48..4a06c7e8 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -70,7 +70,7 @@ static void key_fix_dtype_byteorder(struct expr *key)
 	if (dtype->byteorder == key->byteorder)
 		return;
 
-	key->dtype = set_datatype_alloc(dtype, key->byteorder);
+	datatype_set(key, set_datatype_alloc(dtype, key->byteorder));
 	if (dtype->flags & DTYPE_F_ALLOC)
 		concat_type_destroy(dtype);
 }
@@ -229,7 +229,7 @@ static int expr_evaluate_symbol(struct eval_ctx *ctx, struct expr **expr)
 
 	switch ((*expr)->symtype) {
 	case SYMBOL_VALUE:
-		(*expr)->dtype = ctx->ectx.dtype;
+		datatype_set(*expr, ctx->ectx.dtype);
 		erec = symbol_parse(*expr, &new);
 		if (erec != NULL) {
 			erec_queue(erec, ctx->msgs);
@@ -312,7 +312,7 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp)
 
 	prefix = prefix_expr_alloc(&expr->location, value,
 				   datalen * BITS_PER_BYTE);
-	prefix->dtype = ctx->ectx.dtype;
+	datatype_set(prefix, ctx->ectx.dtype);
 	prefix->flags |= EXPR_F_CONSTANT;
 	prefix->byteorder = BYTEORDER_HOST_ENDIAN;
 
@@ -489,7 +489,7 @@ static int __expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
 	struct expr *expr = *exprp;
 
 	if (expr->exthdr.flags & NFT_EXTHDR_F_PRESENT)
-		expr->dtype = &boolean_type;
+		datatype_set(expr, &boolean_type);
 
 	if (expr_evaluate_primary(ctx, exprp) < 0)
 		return -1;
@@ -915,7 +915,7 @@ static int expr_evaluate_range(struct eval_ctx *ctx, struct expr **expr)
 		return expr_error(ctx->msgs, range,
 				  "Range has zero or negative size");
 
-	range->dtype = left->dtype;
+	datatype_set(range, left->dtype);
 	range->flags |= EXPR_F_CONSTANT;
 	return 0;
 }
@@ -1172,7 +1172,7 @@ static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr,
 	}
 
 	(*expr)->flags |= flags;
-	(*expr)->dtype = concat_type_alloc(ntype);
+	datatype_set(*expr, concat_type_alloc(ntype));
 	(*expr)->len   = (*expr)->dtype->size;
 
 	if (off > 0)
@@ -1239,7 +1239,7 @@ static int expr_evaluate_set_elem(struct eval_ctx *ctx, struct expr **expr)
 		}
 	}
 
-	elem->dtype = elem->key->dtype;
+	datatype_set(elem, 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;
+	datatype_set(set, ctx->ectx.dtype);
 	set->len   = ctx->ectx.len;
 	set->flags |= EXPR_F_CONSTANT;
 	return 0;
@@ -1311,15 +1311,16 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 	switch (map->mappings->etype) {
 	case EXPR_SET:
 		key = constant_expr_alloc(&map->location,
-				 ctx->ectx.dtype,
-				 ctx->ectx.byteorder,
-				 ctx->ectx.len, NULL);
+					  ctx->ectx.dtype,
+					  ctx->ectx.byteorder,
+					  ctx->ectx.len, NULL);
 
 		mappings = implicit_set_declaration(ctx, "__map%d",
 						    key,
 						    mappings);
-		mappings->set->datatype = set_datatype_alloc(ectx.dtype,
-							     ectx.byteorder);
+		mappings->set->datatype =
+			datatype_get(set_datatype_alloc(ectx.dtype,
+							ectx.byteorder));
 		mappings->set->datalen  = ectx.len;
 
 		map->mappings = mappings;
@@ -1356,7 +1357,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 					 map->mappings->set->key->dtype->desc,
 					 map->map->dtype->desc);
 
-	map->dtype = map->mappings->set->datatype;
+	datatype_set(map, map->mappings->set->datatype);
 	map->flags |= EXPR_F_CONSTANT;
 
 	/* Data for range lookups needs to be in big endian order */
@@ -1413,7 +1414,7 @@ static void expr_dtype_integer_compatible(struct eval_ctx *ctx,
 	if (ctx->ectx.dtype &&
 	    ctx->ectx.dtype->basetype == &integer_type &&
 	    ctx->ectx.len == 4 * BITS_PER_BYTE) {
-		expr->dtype = ctx->ectx.dtype;
+		datatype_set(expr, ctx->ectx.dtype);
 		expr->len   = ctx->ectx.len;
 	}
 }
@@ -1554,7 +1555,7 @@ static void binop_transfer_handle_lhs(struct expr **expr)
 	case OP_LSHIFT:
 	case OP_XOR:
 		tmp = expr_get(left->left);
-		tmp->dtype = left->dtype;
+		datatype_set(tmp, left->dtype);
 		expr_free(left);
 		*expr = tmp;
 		break;
@@ -1745,7 +1746,7 @@ static int expr_evaluate_fib(struct eval_ctx *ctx, struct expr **exprp)
 
 	if (expr->flags & EXPR_F_BOOLEAN) {
 		expr->fib.flags |= NFTA_FIB_F_PRESENT;
-		expr->dtype = &boolean_type;
+		datatype_set(expr, &boolean_type);
 	}
 	return expr_evaluate_primary(ctx, exprp);
 }
@@ -2965,7 +2966,7 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt)
 					 map->mappings->set->key->dtype->desc,
 					 map->map->dtype->desc);
 
-	map->dtype = map->mappings->set->datatype;
+	datatype_set(map, map->mappings->set->datatype);
 	map->flags |= EXPR_F_CONSTANT;
 
 	/* Data for range lookups needs to be in big endian order */