Merge branch 'reqid-refcount'

This fixes issues with CHILD_SAs getting reestablished concurrently.
We intend to reuse the reqid of the previous CHILD_SA, however, previously
the reqids were released and up for reassignment to any other CHILD_SA
or trap policy.  This could cause the reqid to get associated with
completely different traffic selectors, as the reestablished CHILD_SA
would eventually get the requested reqid because the traffic selectors
explicitly don't have to match (to allow narrowing for CHILD_SAs based
on trap policies).

Closes strongswan/strongswan#1855

src/libcharon/kernel/kernel_interface.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2019 Tobias Brunner
+ * Copyright (C) 2008-2023 Tobias Brunner
  * Copyright (C) 2010 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -232,9 +232,9 @@ static void reqid_entry_destroy(reqid_entry_t *entry)
 }
 
 /**
- * Hash the shared data of reqid entries
+ * Hash the additional selector properties of reqid entries
  */
-static u_int entry_hash_shared(reqid_entry_t *entry)
+static u_int entry_hash_selectors(reqid_entry_t *entry)
 {
 	u_int hash;
 
@@ -250,9 +250,9 @@ static u_int entry_hash_shared(reqid_entry_t *entry)
 }
 
 /**
- * Compare the shared properties of reqid entries
+ * Compare the additional selector properties of reqid entries
  */
-static bool entry_equals_shared(reqid_entry_t *a, reqid_entry_t *b)
+static bool entry_equals_selectors(reqid_entry_t *a, reqid_entry_t *b)
 {
 	return a->mark_in.value == b->mark_in.value &&
 		   a->mark_in.mask == b->mark_in.mask &&
@@ -268,8 +268,7 @@ static bool entry_equals_shared(reqid_entry_t *a, reqid_entry_t *b)
  */
 static u_int hash_reqid(reqid_entry_t *entry)
 {
-	return chunk_hash_inc(chunk_from_thing(entry->reqid),
+	return chunk_hash(chunk_from_thing(entry->reqid));
-				entry_hash_shared(entry));
 }
 
 /**
@@ -277,11 +276,7 @@ static u_int hash_reqid(reqid_entry_t *entry)
  */
 static bool equals_reqid(reqid_entry_t *a, reqid_entry_t *b)
 {
-	if (a->reqid == b->reqid)
+	return a->reqid == b->reqid;
-	{
-		return entry_equals_shared(a, b);
-	}
-	return FALSE;
 }
 
 /**
@@ -309,7 +304,7 @@ static u_int hash_reqid_by_ts(reqid_entry_t *entry)
 {
 	return hash_ts_array(entry->local,
 				hash_ts_array(entry->remote,
-						 entry_hash_shared(entry)));
+						 entry_hash_selectors(entry)));
 }
 
 /**
@@ -346,7 +341,7 @@ static bool equals_reqid_by_ts(reqid_entry_t *a, reqid_entry_t *b)
 	if (ts_array_equals(a->local, b->local) &&
 		ts_array_equals(a->remote, b->remote))
 	{
-		return entry_equals_shared(a, b);
+		return entry_equals_selectors(a, b);
 	}
 	return FALSE;
 }
@@ -397,16 +392,17 @@ METHOD(kernel_interface_t, alloc_reqid, status_t,
 		/* search by reqid if given */
 		entry = this->reqids->get(this->reqids, tmpl);
 	}
-	if (entry)
+	if (entry && entry_equals_selectors(entry, tmpl))
 	{
-		/* we don't require a traffic selector match for explicit reqids,
+		/* we don't require a traffic selector match for existing reqids,
 		 * as we want to reuse a reqid for trap-triggered policies that
-		 * got narrowed during negotiation. */
+		 * got narrowed during negotiation, but we don't want to reuse the
+		 * reqid if the additional selectors (e.g. marks) are different */
 		reqid_entry_destroy(tmpl);
 	}
 	else
 	{
-		/* search by traffic selectors */
+		/* search by traffic and other selectors */
 		entry = this->reqids_by_ts->get(this->reqids_by_ts, tmpl);
 		if (entry)
 		{
@@ -437,46 +433,42 @@ METHOD(kernel_interface_t, alloc_reqid, status_t,
 	return SUCCESS;
 }
 
-METHOD(kernel_interface_t, release_reqid, status_t,
+METHOD(kernel_interface_t, ref_reqid, status_t,
-	private_kernel_interface_t *this, uint32_t reqid,
+	private_kernel_interface_t *this, uint32_t reqid)
-	mark_t mark_in, mark_t mark_out, uint32_t if_id_in, uint32_t if_id_out,
-	sec_label_t *label)
 {
 	reqid_entry_t *entry, tmpl = {
 		.reqid = reqid,
-		.mark_in = mark_in,
-		.mark_out = mark_out,
-		.if_id_in = if_id_in,
-		.if_id_out = if_id_out,
-		.label = label,
 	};
 
 	this->mutex->lock(this->mutex);
-	entry = this->reqids->remove(this->reqids, &tmpl);
+	entry = this->reqids->get(this->reqids, &tmpl);
 	if (entry)
 	{
-		if (--entry->refs == 0)
+		entry->refs++;
-		{
-			array_insert_create_value(&this->released_reqids, sizeof(uint32_t),
-									  ARRAY_TAIL, &entry->reqid);
-			entry = this->reqids_by_ts->remove(this->reqids_by_ts, entry);
-			if (entry)
-			{
-				reqid_entry_destroy(entry);
-			}
-		}
-		else
-		{
-			this->reqids->put(this->reqids, entry, entry);
-		}
 	}
 	this->mutex->unlock(this->mutex);
+	return entry ? SUCCESS : NOT_FOUND;
+}
 
-	if (entry)
+METHOD(kernel_interface_t, release_reqid, status_t,
+	private_kernel_interface_t *this, uint32_t reqid)
+{
+	reqid_entry_t *entry, tmpl = {
+		.reqid = reqid,
+	};
+
+	this->mutex->lock(this->mutex);
+	entry = this->reqids->get(this->reqids, &tmpl);
+	if (entry && --entry->refs == 0)
 	{
-		return SUCCESS;
+		array_insert_create_value(&this->released_reqids, sizeof(uint32_t),
+								  ARRAY_TAIL, &entry->reqid);
+		this->reqids->remove(this->reqids, entry);
+		this->reqids_by_ts->remove(this->reqids_by_ts, entry);
+		reqid_entry_destroy(entry);
 	}
-	return NOT_FOUND;
+	this->mutex->unlock(this->mutex);
+	return entry ? SUCCESS : NOT_FOUND;
 }
 
 METHOD(kernel_interface_t, add_sa, status_t,
@@ -1064,6 +1056,7 @@ kernel_interface_t *kernel_interface_create()
 			.get_spi = _get_spi,
 			.get_cpi = _get_cpi,
 			.alloc_reqid = _alloc_reqid,
+			.ref_reqid = _ref_reqid,
 			.release_reqid = _release_reqid,
 			.add_sa = _add_sa,
 			.update_sa = _update_sa,

src/libcharon/kernel/kernel_interface.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2016 Tobias Brunner
+ * Copyright (C) 2006-2023 Tobias Brunner
  * Copyright (C) 2006 Daniel Roethlisberger
  * Copyright (C) 2005-2006 Martin Willi
  * Copyright (C) 2005 Jan Hutter
@@ -158,21 +158,25 @@ struct kernel_interface_t {
 							uint32_t if_id_out, sec_label_t *label,
 							uint32_t *reqid);
 
+	/**
+	 * Increase the reference count for the given reqid that was previously
+	 * allocated by alloc_reqid().
+	 *
+	 * The reference must be released with a call to release_reqid().
+	 *
+	 * @param reqid		previously allocated reqid
+	 * @return			SUCCESS if refcount increased, NOT_FOUND if reqid is
+	 *					unknown (shouldn't happen)
+	 */
+	status_t (*ref_reqid)(kernel_interface_t *this, uint32_t reqid);
+
 	/**
 	 * Release a previously allocated reqid.
 	 *
 	 * @param reqid		reqid to release
-	 * @param mark_in	inbound mark on SA
-	 * @param mark_out	outbound mark on SA
-	 * @param if_id_in	inbound interface ID on SA
-	 * @param if_id_out	outbound interface ID on SA
-	 * @param label		security label (usually the one on the policy, not SA)
 	 * @return			SUCCESS if reqid released
 	 */
-	status_t (*release_reqid)(kernel_interface_t *this, uint32_t reqid,
+	status_t (*release_reqid)(kernel_interface_t *this, uint32_t reqid);
-							  mark_t mark_in, mark_t mark_out,
-							  uint32_t if_id_in, uint32_t if_id_out,
-							  sec_label_t *label);
 
 	/**
 	 * Add an SA to the SAD.

src/libcharon/sa/child_sa.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2019 Tobias Brunner
+ * Copyright (C) 2006-2023 Tobias Brunner
  * Copyright (C) 2016 Andreas Steffen
  * Copyright (C) 2005-2008 Martin Willi
  * Copyright (C) 2006 Daniel Roethlisberger
@@ -361,6 +361,17 @@ METHOD(child_sa_t, get_reqid, uint32_t,
 	return this->reqid;
 }
 
+METHOD(child_sa_t, get_reqid_ref, uint32_t,
+	   private_child_sa_t *this)
+{
+	if ((this->reqid_allocated || (!this->static_reqid && this->reqid)) &&
+		charon->kernel->ref_reqid(charon->kernel, this->reqid) == SUCCESS)
+	{
+		return this->reqid;
+	}
+	return 0;
+}
+
 METHOD(child_sa_t, get_unique_id, uint32_t,
 	private_child_sa_t *this)
 {
@@ -842,6 +853,51 @@ METHOD(child_sa_t, alloc_cpi, uint16_t,
 	return 0;
 }
 
+/**
+ * Allocate a reqid for the given local and remote traffic selector lists.
+ * On success, release the previously allocated reqid.
+ */
+static status_t alloc_reqid_lists(private_child_sa_t *this,
+								  linked_list_t *my_ts, linked_list_t *other_ts,
+								  uint32_t *reqid)
+{
+	uint32_t existing_reqid = *reqid;
+	status_t status;
+
+	status = charon->kernel->alloc_reqid(
+							charon->kernel, my_ts, other_ts,
+							this->mark_in, this->mark_out, this->if_id_in,
+							this->if_id_out, label_for(this, LABEL_USE_REQID),
+							reqid);
+
+	if (status == SUCCESS && existing_reqid)
+	{
+		if (charon->kernel->release_reqid(charon->kernel,
+										  existing_reqid) != SUCCESS)
+		{
+			DBG1(DBG_CHD, "releasing previous reqid %u failed", existing_reqid);
+		}
+	}
+	return status;
+}
+
+/**
+ * Allocate a reqid for the given local and remote traffic selectors.
+ */
+static status_t alloc_reqid(private_child_sa_t *this, array_t *my_ts,
+							array_t *other_ts, uint32_t *reqid)
+{
+	linked_list_t *my_ts_list, *other_ts_list;
+	status_t status;
+
+	my_ts_list = linked_list_create_from_enumerator(array_create_enumerator(my_ts));
+	other_ts_list = linked_list_create_from_enumerator(array_create_enumerator(other_ts));
+	status = alloc_reqid_lists(this, my_ts_list, other_ts_list, reqid);
+	my_ts_list->destroy(my_ts_list);
+	other_ts_list->destroy(other_ts_list);
+	return status;
+}
+
 /**
  * Install the given SA in the kernel
  */
@@ -923,10 +979,7 @@ static status_t install_internal(private_child_sa_t *this, chunk_t encr,
 
 	if (!this->reqid_allocated && !this->static_reqid)
 	{
-		status = charon->kernel->alloc_reqid(charon->kernel, my_ts, other_ts,
+		status = alloc_reqid_lists(this, my_ts, other_ts, &this->reqid);
-								this->mark_in, this->mark_out, this->if_id_in,
-								this->if_id_out, label_for(this, LABEL_USE_REQID),
-								&this->reqid);
 		if (status != SUCCESS)
 		{
 			my_ts->destroy(my_ts);
@@ -1321,27 +1374,6 @@ METHOD(child_sa_t, set_policies, void,
 	array_sort(this->other_ts, (void*)traffic_selector_cmp, NULL);
 }
 
-/**
- * Allocate a reqid for the given local and remote traffic selectors.
- */
-static status_t alloc_reqid(private_child_sa_t *this, array_t *my_ts,
-							array_t *other_ts, uint32_t *reqid)
-{
-	linked_list_t *my_ts_list, *other_ts_list;
-	status_t status;
-
-	my_ts_list = linked_list_create_from_enumerator(array_create_enumerator(my_ts));
-	other_ts_list = linked_list_create_from_enumerator(array_create_enumerator(other_ts));
-	status = charon->kernel->alloc_reqid(
-							charon->kernel, my_ts_list, other_ts_list,
-							this->mark_in, this->mark_out, this->if_id_in,
-							this->if_id_out, label_for(this, LABEL_USE_REQID),
-							reqid);
-	my_ts_list->destroy(my_ts_list);
-	other_ts_list->destroy(other_ts_list);
-	return status;
-}
-
 METHOD(child_sa_t, install_policies, status_t,
 	   private_child_sa_t *this)
 {
@@ -1810,9 +1842,7 @@ METHOD(child_sa_t, update, status_t,
 		{
 			if (new_reqid &&
 				charon->kernel->release_reqid(charon->kernel,
-						new_reqid, this->mark_in, this->mark_out,
+											  new_reqid) != SUCCESS)
-						this->if_id_in, this->if_id_out,
-						label_for(this, LABEL_USE_REQID)) != SUCCESS)
 			{
 				DBG1(DBG_CHD, "releasing reqid %u failed", new_reqid);
 			}
@@ -1827,9 +1857,7 @@ METHOD(child_sa_t, update, status_t,
 		if (new_reqid)
 		{
 			if (charon->kernel->release_reqid(charon->kernel,
-						this->reqid, this->mark_in, this->mark_out,
+											  this->reqid) != SUCCESS)
-						this->if_id_in, this->if_id_out,
-						label_for(this, LABEL_USE_REQID)) != SUCCESS)
 			{
 				DBG1(DBG_CHD, "releasing reqid %u failed", this->reqid);
 			}
@@ -1947,12 +1975,10 @@ METHOD(child_sa_t, destroy, void,
 		charon->kernel->del_sa(charon->kernel, &id, &sa);
 	}
 
-	if (this->reqid_allocated)
+	if (this->reqid_allocated || (!this->static_reqid && this->reqid))
 	{
 		if (charon->kernel->release_reqid(charon->kernel,
-						this->reqid, this->mark_in, this->mark_out,
+										  this->reqid) != SUCCESS)
-						this->if_id_in, this->if_id_out,
-						label_for(this, LABEL_USE_REQID)) != SUCCESS)
 		{
 			DBG1(DBG_CHD, "releasing reqid %u failed", this->reqid);
 		}
@@ -2018,6 +2044,7 @@ child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
 		.public = {
 			.get_name = _get_name,
 			.get_reqid = _get_reqid,
+			.get_reqid_ref = _get_reqid_ref,
 			.get_unique_id = _get_unique_id,
 			.get_config = _get_config,
 			.get_state = _get_state,
@@ -2132,7 +2159,11 @@ child_sa_t *child_sa_create(host_t *me, host_t *other, child_cfg_t *config,
 		 * replace the temporary SA on the kernel level. Rekeying such an SA
 		 * requires an explicit reqid, as the cache currently knows the original
 		 * selectors only for that reqid. */
-		this->reqid = data->reqid;
+		if (data->reqid &&
+			charon->kernel->ref_reqid(charon->kernel, data->reqid) == SUCCESS)
+		{
+			this->reqid = data->reqid;
+		}
 	}
 	else
 	{

src/libcharon/sa/child_sa.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006-2019 Tobias Brunner
+ * Copyright (C) 2006-2023 Tobias Brunner
  * Copyright (C) 2006-2008 Martin Willi
  * Copyright (C) 2006 Daniel Roethlisberger
  *
@@ -174,6 +174,18 @@ struct child_sa_t {
 	 */
 	uint32_t (*get_reqid)(child_sa_t *this);
 
+	/**
+	 * Get an additional reference to the allocated reqid of this CHILD SA.
+	 *
+	 * For static reqids or until the reqid is allocated (if none was passed
+	 * in the constructor), this returns 0. The returned reqid must be released
+	 * via kernel_interface_t::release_reqid().
+	 *
+	 * @return 			allocated reqid of the CHILD SA, 0 if reqid is static or
+	 *					not allocated yet
+	 */
+	uint32_t (*get_reqid_ref)(child_sa_t *this);
+
 	/**
 	 * Get the unique numerical identifier for this CHILD_SA.
 	 *

src/libcharon/sa/ike_sa.c

@@ -2067,7 +2067,7 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
 		if (action & ACTION_START)
 		{
 			child_init_args_t args = {
-				.reqid = child_sa->get_reqid(child_sa),
+				.reqid = child_sa->get_reqid_ref(child_sa),
 				.label = child_sa->get_label(child_sa),
 			};
 			child_cfg = child_sa->get_config(child_sa);
@@ -2076,6 +2076,10 @@ static status_t reestablish_children(private_ike_sa_t *this, ike_sa_t *new,
 			other->task_manager->queue_child(other->task_manager,
 											 child_cfg->get_ref(child_cfg),
 											 &args);
+			if (args.reqid)
+			{
+				charon->kernel->release_reqid(charon->kernel, args.reqid);
+			}
 		}
 	}
 	enumerator->destroy(enumerator);

src/libcharon/sa/ikev1/task_manager_v1.c

@@ -1791,6 +1791,7 @@ METHOD(task_manager_t, queue_child_rekey, void,
 	child_sa_t *child_sa;
 	child_cfg_t *cfg;
 	quick_mode_t *task;
+	uint32_t reqid;
 
 	child_sa = this->ike_sa->get_child_sa(this->ike_sa, protocol, spi, TRUE);
 	if (!child_sa)
@@ -1816,7 +1817,12 @@ METHOD(task_manager_t, queue_child_rekey, void,
 			cfg = child_sa->get_config(child_sa);
 			task = quick_mode_create(this->ike_sa, cfg->get_ref(cfg),
 				get_first_ts(child_sa, TRUE), get_first_ts(child_sa, FALSE));
-			task->use_reqid(task, child_sa->get_reqid(child_sa));
+			reqid = child_sa->get_reqid_ref(child_sa);
+			if (reqid)
+			{
+				task->use_reqid(task, reqid);
+				charon->kernel->release_reqid(charon->kernel, reqid);
+			}
 			task->use_marks(task, child_sa->get_mark(child_sa, TRUE).value,
 							child_sa->get_mark(child_sa, FALSE).value);
 			task->use_if_ids(task, child_sa->get_if_id(child_sa, TRUE),

src/libcharon/sa/ikev1/tasks/quick_delete.c

@@ -151,7 +151,7 @@ static status_t delete_child(private_quick_delete_t *this,
 		if (remote_close)
 		{
 			child_init_args_t args = {
-				.reqid = child_sa->get_reqid(child_sa),
+				.reqid = child_sa->get_reqid_ref(child_sa),
 			};
 			action_t action;
 
@@ -169,6 +169,10 @@ static status_t delete_child(private_quick_delete_t *this,
 				child_cfg->get_ref(child_cfg);
 				status = this->ike_sa->initiate(this->ike_sa, child_cfg, &args);
 			}
+			if (args.reqid)
+			{
+				charon->kernel->release_reqid(charon->kernel, args.reqid);
+			}
 			child_cfg->destroy(child_cfg);
 		}
 	}

src/libcharon/sa/ikev1/tasks/quick_mode.c

@@ -1432,7 +1432,16 @@ METHOD(quick_mode_t, get_mid, uint32_t,
 METHOD(quick_mode_t, use_reqid, void,
 	private_quick_mode_t *this, uint32_t reqid)
 {
-	this->child.reqid = reqid;
+	uint32_t existing_reqid = this->child.reqid;
+
+	if (!reqid || charon->kernel->ref_reqid(charon->kernel, reqid) == SUCCESS)
+	{
+		this->child.reqid = reqid;
+		if (existing_reqid)
+		{
+			charon->kernel->release_reqid(charon->kernel, existing_reqid);
+		}
+	}
 }
 
 METHOD(quick_mode_t, use_marks, void,
@@ -1496,6 +1505,10 @@ METHOD(task_t, destroy, void,
 	DESTROY_IF(this->child_sa);
 	DESTROY_IF(this->config);
 	DESTROY_IF(this->dh);
+	if (this->child.reqid)
+	{
+		charon->kernel->release_reqid(charon->kernel, this->child.reqid);
+	}
 	free(this);
 }
 

src/libcharon/sa/ikev1/tasks/quick_mode.h

@@ -50,6 +50,10 @@ struct quick_mode_t {
 	/**
 	 * Use a specific reqid to install this CHILD_SA.
 	 *
+	 * This must only be called with dynamically allocated reqids (i.e. from
+	 * kernel_interface_t::alloc_reqid()), the method takes a reference that's
+	 * maintained for the lifetime of the task.
+	 *
 	 * @param reqid			reqid to use
 	 */
 	void (*use_reqid)(quick_mode_t *this, uint32_t reqid);

src/libcharon/sa/ikev2/task_manager_v2.c

@@ -2126,6 +2126,7 @@ static void trigger_mbb_reauth(private_task_manager_t *this)
 	ike_sa_t *new;
 	host_t *host;
 	queued_task_t *queued;
+	uint32_t reqid;
 	bool children = FALSE;
 
 	new = charon->ike_sa_manager->create_new(charon->ike_sa_manager,
@@ -2165,7 +2166,12 @@ static void trigger_mbb_reauth(private_task_manager_t *this)
 		cfg = child_sa->get_config(child_sa);
 		child_create = child_create_create(new, cfg->get_ref(cfg),
 										   FALSE, NULL, NULL);
-		child_create->use_reqid(child_create, child_sa->get_reqid(child_sa));
+		reqid = child_sa->get_reqid_ref(child_sa);
+		if (reqid)
+		{
+			child_create->use_reqid(child_create, reqid);
+			charon->kernel->release_reqid(charon->kernel, reqid);
+		}
 		child_create->use_marks(child_create,
 								child_sa->get_mark(child_sa, TRUE).value,
 								child_sa->get_mark(child_sa, FALSE).value);

src/libcharon/sa/ikev2/tasks/child_create.c

@@ -1925,7 +1925,16 @@ METHOD(task_t, process_i, status_t,
 METHOD(child_create_t, use_reqid, void,
 	private_child_create_t *this, uint32_t reqid)
 {
-	this->child.reqid = reqid;
+	uint32_t existing_reqid = this->child.reqid;
+
+	if (!reqid || charon->kernel->ref_reqid(charon->kernel, reqid) == SUCCESS)
+	{
+		this->child.reqid = reqid;
+		if (existing_reqid)
+		{
+			charon->kernel->release_reqid(charon->kernel, existing_reqid);
+		}
+	}
 }
 
 METHOD(child_create_t, use_marks, void,
@@ -2064,6 +2073,10 @@ METHOD(task_t, destroy, void,
 	{
 		DESTROY_IF(this->child_sa);
 	}
+	if (this->child.reqid)
+	{
+		charon->kernel->release_reqid(charon->kernel, this->child.reqid);
+	}
 	DESTROY_IF(this->packet_tsi);
 	DESTROY_IF(this->packet_tsr);
 	DESTROY_IF(this->proposal);

src/libcharon/sa/ikev2/tasks/child_create.h

@@ -49,6 +49,10 @@ struct child_create_t {
 	 * When this task is used for rekeying, the same reqid is used
 	 * for the new CHILD_SA.
 	 *
+	 * This must only be called with dynamically allocated reqids (i.e. from
+	 * kernel_interface_t::alloc_reqid()), the method takes a reference that's
+	 * maintained for the lifetime of the task.
+	 *
 	 * @param reqid		reqid to use
 	 */
 	void (*use_reqid) (child_create_t *this, uint32_t reqid);

src/libcharon/sa/ikev2/tasks/child_delete.c

@@ -366,7 +366,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
 		spi = child_sa->get_spi(child_sa, TRUE);
 		child_cfg = child_sa->get_config(child_sa);
 		child_cfg->get_ref(child_cfg);
-		args.reqid = child_sa->get_reqid(child_sa);
+		args.reqid = child_sa->get_reqid_ref(child_sa);
 		args.label = child_sa->get_label(child_sa);
 		if (args.label)
 		{
@@ -391,6 +391,10 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
 			}
 		}
 		child_cfg->destroy(child_cfg);
+		if (args.reqid)
+		{
+			charon->kernel->release_reqid(charon->kernel, args.reqid);
+		}
 		DESTROY_IF(args.label);
 		if (status != SUCCESS)
 		{

src/libcharon/sa/ikev2/tasks/child_rekey.c

@@ -213,8 +213,12 @@ METHOD(task_t, build_i, status_t,
 			this->child_create->use_dh_group(this->child_create, dh_group);
 		}
 	}
-	reqid = this->child_sa->get_reqid(this->child_sa);
+	reqid = this->child_sa->get_reqid_ref(this->child_sa);
-	this->child_create->use_reqid(this->child_create, reqid);
+	if (reqid)
+	{
+		this->child_create->use_reqid(this->child_create, reqid);
+		charon->kernel->release_reqid(charon->kernel, reqid);
+	}
 	this->child_create->use_marks(this->child_create,
 						this->child_sa->get_mark(this->child_sa, TRUE).value,
 						this->child_sa->get_mark(this->child_sa, FALSE).value);
@@ -282,8 +286,12 @@ METHOD(task_t, build_r, status_t,
 	}
 
 	/* let the CHILD_CREATE task build the response */
-	reqid = this->child_sa->get_reqid(this->child_sa);
+	reqid = this->child_sa->get_reqid_ref(this->child_sa);
-	this->child_create->use_reqid(this->child_create, reqid);
+	if (reqid)
+	{
+		this->child_create->use_reqid(this->child_create, reqid);
+		charon->kernel->release_reqid(charon->kernel, reqid);
+	}
 	this->child_create->use_marks(this->child_create,
 						this->child_sa->get_mark(this->child_sa, TRUE).value,
 						this->child_sa->get_mark(this->child_sa, FALSE).value);
@@ -430,7 +438,7 @@ METHOD(task_t, process_i, status_t,
 		protocol = this->child_sa->get_protocol(this->child_sa);
 		child_cfg = this->child_sa->get_config(this->child_sa);
 		child_cfg->get_ref(child_cfg);
-		args.reqid = this->child_sa->get_reqid(this->child_sa);
+		args.reqid = this->child_sa->get_reqid_ref(this->child_sa);
 		args.label = this->child_sa->get_label(this->child_sa);
 		if (args.label)
 		{
@@ -440,6 +448,10 @@ METHOD(task_t, process_i, status_t,
 		this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi);
 		status = this->ike_sa->initiate(this->ike_sa,
 										child_cfg->get_ref(child_cfg), &args);
+		if (args.reqid)
+		{
+			charon->kernel->release_reqid(charon->kernel, args.reqid);
+		}
 		DESTROY_IF(args.label);
 		return status;
 	}

src/libcharon/sa/trap_manager.c

@@ -524,6 +524,7 @@ METHOD(trap_manager_t, acquire, void,
 	child_cfg_t *child;
 	ike_sa_t *ike_sa;
 	host_t *host;
+	uint32_t allocated_reqid;
 	bool wildcard, ignore = FALSE;
 
 	this->lock->read_lock(this->lock);
@@ -596,6 +597,8 @@ METHOD(trap_manager_t, acquire, void,
 	peer = found->peer_cfg->get_ref(found->peer_cfg);
 	child = found->child_sa->get_config(found->child_sa);
 	child = child->get_ref(child);
+	/* only pass allocated reqids explicitly, take a reference */
+	allocated_reqid = found->child_sa->get_reqid_ref(found->child_sa);
 	/* don't hold the lock while checking out the IKE_SA */
 	this->lock->unlock(this->lock);
 
@@ -635,7 +638,7 @@ METHOD(trap_manager_t, acquire, void,
 	if (ike_sa)
 	{
 		child_init_args_t args = {
-			.reqid = reqid,
+			.reqid = allocated_reqid,
 			.src = data->src,
 			.dst = data->dst,
 			.label = data->label,
@@ -669,6 +672,10 @@ METHOD(trap_manager_t, acquire, void,
 		destroy_acquire(acquire);
 		child->destroy(child);
 	}
+	if (allocated_reqid)
+	{
+		charon->kernel->release_reqid(charon->kernel, allocated_reqid);
+	}
 }
 
 /**