From: Ondřej Kuzník <ondra@mistotebe.net>
Date: Mon, 12 Jun 2023 10:42:55 +0000 (+0100)
Subject: ITS#10013 Make freeing controls the job of whoever added it
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=860b61f41dfeeb19cc0eb011f290561c68a13de3;p=thirdparty%2Fopenldap.git

ITS#10013 Make freeing controls the job of whoever added it
---

diff --git a/contrib/slapd-modules/dupent/dupent.c b/contrib/slapd-modules/dupent/dupent.c
index 76af313e94..7f5aeed3d4 100644
--- a/contrib/slapd-modules/dupent/dupent.c
+++ b/contrib/slapd-modules/dupent/dupent.c
@@ -52,6 +52,8 @@
 static int dupent_cid;
 static slap_overinst dupent;
 
+static const char dupent_response_oid[] = LDAP_CONTROL_DUPENT_RESPONSE;
+
 typedef struct dupent_t {
 	AttributeName	*ds_an;
 	ber_len_t		ds_nattrs;
@@ -273,7 +275,7 @@ dupent_response_done( Operation *op, SlapReply *rs )
 		sizeof( LDAPControl ) + ctrlval.bv_len + 1,
 		op->o_tmpmemctx );
 	ctrl->ldctl_value.bv_val = (char *)&ctrl[ 1 ];
-	ctrl->ldctl_oid = LDAP_CONTROL_DUPENT_RESPONSE;
+	ctrl->ldctl_oid = dupent_response_oid;
 	ctrl->ldctl_iscritical = 0;
 	ctrl->ldctl_value.bv_len = ctrlval.bv_len;
 	AC_MEMCPY( ctrl->ldctl_value.bv_val, ctrlval.bv_val, ctrlval.bv_len );
@@ -295,11 +297,13 @@ dupent_response_entry_1level(
 	int nattrs,
 	int level )
 {
+	LDAPControl ctrl = {
+		.ldctl_oid = LDAP_CONTROL_DUPENT_ENTRY,
+		.ldctl_iscritical = 0
+	};
 	int i, rc = LDAP_SUCCESS;
 
 	for ( i = 0; i < valnum[level].ap->a_numvals; i++ ) {
-		LDAPControl	*ctrl;
-
 		valnum[level].a.a_vals[0] = valnum[level].ap->a_vals[i];
 		if ( valnum[level].ap->a_nvals != valnum[level].ap->a_vals ) {
 			valnum[level].a.a_nvals[0] = valnum[level].ap->a_nvals[i];
@@ -315,15 +319,7 @@ dupent_response_entry_1level(
 			continue;
 		}
 
-		/* NOTE: add the control all times, under the assumption
-		 * send_search_entry() honors the REP_CTRLS_MUSTBEFREED
-		 * set by slap_add_ctrls(); this is not true (ITS#6629)
-		 */
-		ctrl = op->o_tmpcalloc( 1, sizeof( LDAPControl ), op->o_tmpmemctx );
-		ctrl->ldctl_oid = LDAP_CONTROL_DUPENT_ENTRY;
-		ctrl->ldctl_iscritical = 0;
-
-		slap_add_ctrl( op, rs, ctrl );
+		slap_add_ctrl( op, rs, &ctrl );
 
 		/* do the real send */
 		rs->sr_entry = e;
@@ -434,11 +430,6 @@ dupent_response_entry( Operation *op, SlapReply *rs )
 
 	*app = &valnum[0].a;
 
-	/* NOTE: since send_search_entry() does not honor the
-	 * REP_CTRLS_MUSTBEFREED flag set by slap_add_ctrls(),
-	 * the control could be added here once for all (ITS#6629)
-	 */
-
 	dc->dc_skip = 1;
 	rc = dupent_response_entry_1level( op, rs, e, valnum, nattrs, 0 );
 	dc->dc_skip = 0;
@@ -489,6 +480,22 @@ dupent_cleanup( Operation *op, SlapReply *rs )
 		op->o_ctrldupent = NULL;
 	}
 
+	if ( rs->sr_ctrls ) {
+		int n;
+
+		for ( n = 0; rs->sr_ctrls[n]; n++ ) {
+			/* We only add one control */
+			if ( rs->sr_ctrls[n]->ldctl_oid == dupent_response_oid ) {
+				op->o_tmpfree( rs->sr_ctrls[n], op->o_tmpmemctx );
+				break;
+			}
+		}
+
+		for ( ; rs->sr_ctrls[n]; n++ ) {
+			rs->sr_ctrls[n] = rs->sr_ctrls[n+1];
+		}
+	}
+
 	return SLAP_CB_CONTINUE;
 }
 
diff --git a/contrib/slapd-modules/noopsrch/noopsrch.c b/contrib/slapd-modules/noopsrch/noopsrch.c
index 6e0c35b010..cb008d683b 100644
--- a/contrib/slapd-modules/noopsrch/noopsrch.c
+++ b/contrib/slapd-modules/noopsrch/noopsrch.c
@@ -28,6 +28,8 @@
  */
 #define	LDAP_CONTROL_X_NOOPSRCH		"1.3.6.1.4.1.4203.666.5.18"
 
+static const char noopsrch_oid[] = LDAP_CONTROL_X_NOOPSRCH;
+
 #include "slap.h"
 #include "ac/string.h"
 
@@ -128,7 +130,7 @@ noopsrch_response( Operation *op, SlapReply *rs )
 			sizeof( LDAPControl ) + ctrlval.bv_len + 1,
 			op->o_tmpmemctx );
 		ctrl->ldctl_value.bv_val = (char *)&ctrl[ 1 ];
-		ctrl->ldctl_oid = LDAP_CONTROL_X_NOOPSRCH;
+		ctrl->ldctl_oid = noopsrch_oid;
 		ctrl->ldctl_iscritical = 0;
 		ctrl->ldctl_value.bv_len = ctrlval.bv_len;
 		AC_MEMCPY( ctrl->ldctl_value.bv_val, ctrlval.bv_val, ctrlval.bv_len );
@@ -155,6 +157,21 @@ noopsrch_cleanup( Operation *op, SlapReply *rs )
 		op->o_callback = NULL;
 	}
 
+	if ( rs->sr_ctrls ) {
+		int n;
+
+		for ( n = 0; rs->sr_ctrls[n]; n++ ) {
+			if ( rs->sr_ctrls[n]->ldctl_oid == noopsrch_oid ) {
+				op->o_tmpfree( rs->sr_ctrls[n], op->o_tmpmemctx );
+				break;
+			}
+		}
+
+		for ( ; rs->sr_ctrls[n]; n++ ) {
+			rs->sr_ctrls[n] = rs->sr_ctrls[n+1];
+		}
+	}
+
 	return SLAP_CB_CONTINUE;
 }
 
diff --git a/servers/slapd/back-ldap/bind.c b/servers/slapd/back-ldap/bind.c
index 7665cb6ae7..14357ac42c 100644
--- a/servers/slapd/back-ldap/bind.c
+++ b/servers/slapd/back-ldap/bind.c
@@ -1978,7 +1978,6 @@ retry:;
 			rs->sr_flags |= REP_CTRLS_MUSTBEFREED;
 
 		} else {
-			assert( rs->sr_ctrls != NULL );
 			rs->sr_ctrls = NULL;
 		}
 
diff --git a/servers/slapd/back-ldap/search.c b/servers/slapd/back-ldap/search.c
index 90b5b65121..daf4bfe884 100644
--- a/servers/slapd/back-ldap/search.c
+++ b/servers/slapd/back-ldap/search.c
@@ -160,7 +160,7 @@ ldap_back_search(
 	char		**attrs = NULL;
 	int		freetext = 0, filter_undef = 0;
 	int		do_retry = 1, dont_retry = 0;
-	LDAPControl	**ctrls = NULL;
+	LDAPControl	**ctrls = NULL, **res_ctrls = NULL;
 	char		**references = NULL;
 	int		remove_unknown_schema =
 				 LDAP_BACK_OMIT_UNKNOWN_SCHEMA (li);
@@ -374,15 +374,18 @@ retry:
 			rc = ldap_build_entry( op, e, &ent, &bdn,
 						remove_unknown_schema);
 			if ( rc == LDAP_SUCCESS ) {
+
 				ldap_get_entry_controls( lc->lc_ld, res, &rs->sr_ctrls );
+				res_ctrls = rs->sr_ctrls;
+
 				rs->sr_entry = &ent;
 				rs->sr_attrs = op->ors_attrs;
 				rs->sr_operational_attrs = NULL;
 				rs->sr_flags = 0;
 				rs->sr_err = LDAP_SUCCESS;
 				rc = rs->sr_err = send_search_entry( op, rs );
-				if ( rs->sr_ctrls ) {
-					ldap_controls_free( rs->sr_ctrls );
+				if ( res_ctrls ) {
+					ldap_controls_free( res_ctrls );
 					rs->sr_ctrls = NULL;
 				}
 				rs->sr_entry = NULL;
@@ -426,6 +429,7 @@ retry:
 			if ( rc != LDAP_SUCCESS ) {
 				continue;
 			}
+			res_ctrls = rs->sr_ctrls;
 
 			/* FIXME: there MUST be at least one */
 			if ( references && references[ 0 ] && references[ 0 ][ 0 ] ) {
@@ -464,8 +468,8 @@ retry:
 				references = NULL;
 			}
 
-			if ( rs->sr_ctrls ) {
-				ldap_controls_free( rs->sr_ctrls );
+			if ( res_ctrls ) {
+				ldap_controls_free( res_ctrls );
 				rs->sr_ctrls = NULL;
 			}
 
@@ -481,6 +485,7 @@ retry:
 			if ( rc != LDAP_SUCCESS ) {
 				continue;
 			}
+			res_ctrls = rs->sr_ctrls;
 
 			slap_send_ldap_intermediate( op, rs );
 
@@ -494,8 +499,8 @@ retry:
 				rs->sr_rspdata = NULL;
 			}
 
-			if ( rs->sr_ctrls != NULL ) {
-				ldap_controls_free( rs->sr_ctrls );
+			if ( res_ctrls ) {
+				ldap_controls_free( res_ctrls );
 				rs->sr_ctrls = NULL;
 			}
 
@@ -514,6 +519,7 @@ retry:
 				rs->sr_err = rc;
 			}
 			rs->sr_err = slap_map_api2result( rs );
+			res_ctrls = rs->sr_ctrls;
 
 			/* RFC 4511: referrals can only appear
 			 * if result code is LDAP_REFERRAL */
@@ -634,9 +640,10 @@ finish:;
 
 	(void)ldap_back_controls_free( op, rs, &ctrls );
 
-	if ( rs->sr_ctrls ) {
-		ldap_controls_free( rs->sr_ctrls );
+	if ( res_ctrls ) {
+		ldap_controls_free( res_ctrls );
 		rs->sr_ctrls = NULL;
+		res_ctrls = NULL;
 	}
 
 	if ( rs->sr_text ) {
@@ -935,7 +942,7 @@ ldap_back_entry_get(
 	char		*filter = NULL;
 	SlapReply	rs;
 	int		do_retry = 1;
-	LDAPControl	**ctrls = NULL;
+	LDAPControl	**ctrls = NULL, **res_ctrls = NULL;
 	Operation op2 = *op;
 
 	int		remove_unknown_schema =
diff --git a/servers/slapd/back-mdb/search.c b/servers/slapd/back-mdb/search.c
index 927a3204ef..81b4e01882 100644
--- a/servers/slapd/back-mdb/search.c
+++ b/servers/slapd/back-mdb/search.c
@@ -1492,7 +1492,10 @@ send_paged_response(
 	ID		*lastid,
 	int		tentries )
 {
-	LDAPControl	*ctrl;
+	LDAPControl	ctrl = {
+		.ldctl_oid = LDAP_CONTROL_PAGEDRESULTS,
+		.ldctl_iscritical = 0
+	};
 	BerElementBuffer berbuf;
 	BerElement	*ber = (BerElement *)&berbuf;
 	PagedResultsCookie respcookie;
@@ -1522,15 +1525,11 @@ send_paged_response(
 	/* return size of 0 -- no estimate */
 	ber_printf( ber, "{iO}", 0, &cookie ); 
 
-	ctrl = op->o_tmpalloc( sizeof(LDAPControl), op->o_tmpmemctx );
-	if ( ber_flatten2( ber, &ctrl->ldctl_value, 0 ) == -1 ) {
+	if ( ber_flatten2( ber, &ctrl.ldctl_value, 0 ) == -1 ) {
 		goto done;
 	}
 
-	ctrl->ldctl_oid = LDAP_CONTROL_PAGEDRESULTS;
-	ctrl->ldctl_iscritical = 0;
-
-	slap_add_ctrl( op, rs, ctrl );
+	slap_add_ctrl( op, rs, &ctrl );
 	rs->sr_err = LDAP_SUCCESS;
 	send_ldap_result( op, rs );
 
diff --git a/servers/slapd/back-meta/bind.c b/servers/slapd/back-meta/bind.c
index edfabbb440..07062a5910 100644
--- a/servers/slapd/back-meta/bind.c
+++ b/servers/slapd/back-meta/bind.c
@@ -1261,7 +1261,6 @@ retry:;
 		ber_memvfree( (void **)refs );
 	}
 	if ( ctrls ) {
-		assert( rs->sr_ctrls != NULL );
 		ldap_controls_free( ctrls );
 	}
 
diff --git a/servers/slapd/back-meta/search.c b/servers/slapd/back-meta/search.c
index cfd2f4b0c1..397a4bc058 100644
--- a/servers/slapd/back-meta/search.c
+++ b/servers/slapd/back-meta/search.c
@@ -2016,6 +2016,7 @@ meta_send_entry(
 	Entry 			ent = { 0 };
 	BerElement 		ber = *ldap_get_message_ber( e );
 	Attribute 		*attr, **attrp;
+	LDAPControl **res_ctrls;
 	struct berval 		bdn,
 				dn = BER_BVNULL;
 	const char 		*text;
@@ -2396,12 +2397,13 @@ next_attr:;
 	}
 
 	ldap_get_entry_controls( mc->mc_conns[target].msc_ld,
-		e, &rs->sr_ctrls );
+		e, &res_ctrls );
 	rs->sr_entry = &ent;
 	rs->sr_attrs = op->ors_attrs;
 	rs->sr_operational_attrs = NULL;
 	rs->sr_flags = mi->mi_targets[ target ]->mt_rep_flags;
 	rs->sr_err = LDAP_SUCCESS;
+	rs->sr_ctrls = res_ctrls;
 	rc = send_search_entry( op, rs );
 	switch ( rc ) {
 	case LDAP_UNAVAILABLE:
@@ -2412,8 +2414,8 @@ next_attr:;
 done:;
 	rs->sr_entry = NULL;
 	rs->sr_attrs = NULL;
-	if ( rs->sr_ctrls != NULL ) {
-		ldap_controls_free( rs->sr_ctrls );
+	if ( res_ctrls != NULL ) {
+		ldap_controls_free( res_ctrls );
 		rs->sr_ctrls = NULL;
 	}
 	if ( !BER_BVISNULL( &ent.e_name ) ) {
diff --git a/servers/slapd/back-wt/search.c b/servers/slapd/back-wt/search.c
index 41d031a4f1..924179f31d 100644
--- a/servers/slapd/back-wt/search.c
+++ b/servers/slapd/back-wt/search.c
@@ -273,7 +273,7 @@ send_paged_response(
 	ID      *lastid,
 	int     tentries )
 {
-	LDAPControl *ctrl;
+	LDAPControl ctrl;
 	BerElementBuffer berbuf;
 	BerElement  *ber = (BerElement *)&berbuf;
 	PagedResultsCookie respcookie;
@@ -303,15 +303,14 @@ send_paged_response(
 	/* return size of 0 -- no estimate */
 	ber_printf( ber, "{iO}", 0, &cookie );
 
-	ctrl = op->o_tmpalloc( sizeof(LDAPControl), op->o_tmpmemctx );
-	if ( ber_flatten2( ber, &ctrl->ldctl_value, 0 ) == -1 ) {
+	if ( ber_flatten2( ber, &ctrl.ldctl_value, 0 ) == -1 ) {
 		goto done;
 	}
 
-	ctrl->ldctl_oid = LDAP_CONTROL_PAGEDRESULTS;
-	ctrl->ldctl_iscritical = 0;
+	ctrl.ldctl_oid = LDAP_CONTROL_PAGEDRESULTS;
+	ctrl.ldctl_iscritical = 0;
 
-	slap_add_ctrl( op, rs, ctrl );
+	slap_add_ctrl( op, rs, &ctrl );
 	rs->sr_err = LDAP_SUCCESS;
 	send_ldap_result( op, rs );
 
diff --git a/servers/slapd/overlays/deref.c b/servers/slapd/overlays/deref.c
index 51f7625d2e..664536ff15 100644
--- a/servers/slapd/overlays/deref.c
+++ b/servers/slapd/overlays/deref.c
@@ -141,6 +141,8 @@ static int			deref_cid;
 static slap_overinst 		deref;
 static int ov_count;
 
+static const char deref_oid[] = LDAP_CONTROL_X_DEREF;
+
 static int
 deref_parseCtrl (
 	Operation *op,
@@ -275,6 +277,21 @@ deref_cleanup( Operation *op, SlapReply *rs )
 		op->o_ctrlderef = NULL;
 	}
 
+	if ( rs->sr_ctrls ) {
+		int n;
+
+		for ( n = 0; rs->sr_ctrls[n]; n++ ) {
+			if ( rs->sr_ctrls[n]->ldctl_oid == deref_oid ) {
+				op->o_tmpfree( rs->sr_ctrls[n], op->o_tmpmemctx );
+				break;
+			}
+		}
+
+		for ( ; rs->sr_ctrls[n]; n++ ) {
+			rs->sr_ctrls[n] = rs->sr_ctrls[n+1];
+		}
+	}
+
 	return SLAP_CB_CONTINUE;
 }
 
@@ -465,7 +482,7 @@ deref_response( Operation *op, SlapReply *rs )
 			sizeof( LDAPControl ) + ctrlval.bv_len + 1,
 			op->o_tmpmemctx );
 		ctrl->ldctl_value.bv_val = (char *)&ctrl[ 1 ];
-		ctrl->ldctl_oid = LDAP_CONTROL_X_DEREF;
+		ctrl->ldctl_oid = deref_oid;
 		ctrl->ldctl_iscritical = 0;
 		ctrl->ldctl_value.bv_len = ctrlval.bv_len;
 		AC_MEMCPY( ctrl->ldctl_value.bv_val, ctrlval.bv_val, ctrlval.bv_len );
diff --git a/servers/slapd/overlays/sssvlv.c b/servers/slapd/overlays/sssvlv.c
index 92255dd7a6..1334b9dacb 100644
--- a/servers/slapd/overlays/sssvlv.c
+++ b/servers/slapd/overlays/sssvlv.c
@@ -704,6 +704,10 @@ static void send_result(
 		slap_add_ctrls( op, rs, ctrls, i );
 	send_ldap_result( op, rs );
 
+	for ( ; i--; ) {
+		op->o_tmpfree( ctrls[i], op->o_tmpmemctx );
+	}
+
 	if ( so->so_tree == NULL ) {
 		/* Search finished, so clean up */
 		free_sort_op( op->o_conn, so );
@@ -813,10 +817,10 @@ static int sssvlv_op_search(
 	PagedResultsState *ps;
 	vlv_ctrl *vc;
 	int sess_id;
+	LDAPControl *ctrl = NULL;
 
 	if ( op->o_ctrlflag[sss_cid] <= SLAP_CONTROL_IGNORED ) {
 		if ( op->o_ctrlflag[vlv_cid] > SLAP_CONTROL_IGNORED ) {
-			LDAPControl *ctrl;
 			so2.so_vcontext = 0;
 			so2.so_vlv_target = 0;
 			so2.so_nentries = 0;
diff --git a/servers/slapd/result.c b/servers/slapd/result.c
index a928e95c71..c9f9c8a365 100644
--- a/servers/slapd/result.c
+++ b/servers/slapd/result.c
@@ -816,7 +816,7 @@ clean2:;
 	if ( rs->sr_flags & REP_CTRLS_MUSTBEFREED ) {
 		rs->sr_flags ^= REP_CTRLS_MUSTBEFREED; /* paranoia */
 		if ( rs->sr_ctrls ) {
-			slap_free_ctrls( op, rs->sr_ctrls );
+			op->o_tmpfree( rs->sr_ctrls, op->o_tmpmemctx );
 			rs->sr_ctrls = NULL;
 		}
 	}
@@ -1543,7 +1543,7 @@ error_return:;
 	if ( rs->sr_flags & REP_CTRLS_MUSTBEFREED ) {
 		rs->sr_flags ^= REP_CTRLS_MUSTBEFREED; /* paranoia */
 		if ( rs->sr_ctrls ) {
-			slap_free_ctrls( op, rs->sr_ctrls );
+			op->o_tmpfree( rs->sr_ctrls, op->o_tmpmemctx );
 			rs->sr_ctrls = NULL;
 		}
 	}
@@ -1701,7 +1701,7 @@ rel:
 	if ( rs->sr_flags & REP_CTRLS_MUSTBEFREED ) {
 		rs->sr_flags ^= REP_CTRLS_MUSTBEFREED; /* paranoia */
 		if ( rs->sr_ctrls ) {
-			slap_free_ctrls( op, rs->sr_ctrls );
+			op->o_tmpfree( rs->sr_ctrls, op->o_tmpmemctx );
 			rs->sr_ctrls = NULL;
 		}
 	}