patches to make bind9 with TKEY/GSS updates easier to configure

Mark Andrews marka at isc.org
Wed Dec 1 10:39:55 UTC 2010


In message <19701.60603.630642.994047 at samba.org>, tridge at samba.org writes:
> Hi Michael,
> 
>  > There is one problem I haven't solved yet. The test triggers a assert
>  > in named if I leave in the "-T clienttest" option in start.pl. I'm
>  > guessing this is some kind of memory tracer?
>  > 
>  >    01-Dec-2010 00:05:24.681 mem.c:1074: INSIST((((ctx->debuglist[i]).head 
> == ((void *)0)) ? isc_boolean_true : isc_boolean_false)) failed, back trace
> 
> I've been looking at this failure, and it seems to be an existing
> bug. When I run the testsuite against a copy of bind 9.7.2-P2 without
> my changes it fails in the same way. 
> 
> Here is a backtrace of the failure:
> 
> (gdb) bt
> #0  0x00007ffff56dea75 in *__GI_raise (sig=<value optimised out>) at ../nptl/
> sysdeps/unix/sysv/linux/raise.c:64
> #1  0x00007ffff56e25c0 in *__GI_abort () at abort.c:92
> #2  0x0000000000429fd0 in assertion_failed (file=0x7ffff69b99a2 "mem.c", line
> =1074, type=isc_assertiontype_insist, 
>     cond=0x7ffff69b9d48 "(((ctx->debuglist[i]).head == ((void *)0)) ? isc_boo
> lean_true : isc_boolean_false)") at ./main.c:210
> #3  0x00007ffff6976940 in isc_assertion_failed (file=0x7ffff69b99a2 "mem.c", 
> line=1074, type=isc_assertiontype_insist, 
>     cond=0x7ffff69b9d48 "(((ctx->debuglist[i]).head == ((void *)0)) ? isc_boo
> lean_true : isc_boolean_false)") at assertions.c:57
> #4  0x00007ffff6989c69 in destroy (ctx=0xa7d990) at mem.c:1074
> #5  0x00007ffff698a4d5 in isc___mem_putanddetach (ctxp=0xa8cc18, ptr=0x0, siz
> e=1400, file=0x47daef "client.c", line=535) at mem.c:1202
> #6  0x000000000041863b in exit_check (client=0xa8cc10) at client.c:535
> #7  0x00000000004188e5 in client_shutdown (task=0x7ffff7f8ece0, event=0x0) at
>  client.c:609
> #8  0x00007ffff699d8a8 in dispatch (manager=0x7ffff7f7b020) at task.c:1013
> #9  0x00007ffff699db4c in run (uap=0x7ffff7f7b020) at task.c:1158
> #10 0x00007ffff5d849ca in start_thread (arg=<value optimised out>) at pthread
> _create.c:300
> #11 0x00007ffff579170d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone
> .S:112
> #12 0x0000000000000000 in ?? ()
> 
> 
> As far as I can tell, this means there is some error in the debug
> calls that the code is making, probably in the tkey code. What sort of
> rules could the debug code be violation to cause this error?

A memory leak.  The test suite runs with full memory debugging turned
on and debuglist[i] has a linked list of arrays with <address,file,line>
tuples with a pointer to the allocated memory and which file and
line it was allocated at.  When memory is freed the tuple is cleared.
When the array is empty it unlinked and freed.

That said we have been chasing a leak in this area.  See attached.

> Cheers, Tridge
> _______________________________________________
> bind-workers mailing list
> bind-workers at lists.isc.org
> https://lists.isc.org/mailman/listinfo/bind-workers
-- 
Mark Andrews, ISC
1 Seymour St., Dundas Valley, NSW 2117, Australia
PHONE: +61 2 9871 4742                 INTERNET: marka at isc.org
-------------- next part --------------
Index: bind9/bin/dig/dighost.c
diff -u bind9/bin/dig/dighost.c:1.334 bind9/bin/dig/dighost.c:1.334.8.2
--- bind9/bin/dig/dighost.c:1.334	Tue Nov 16 05:38:30 2010
+++ bind9/bin/dig/dighost.c	Tue Nov 30 23:29:50 2010
@@ -252,7 +252,7 @@
 			     char **tempp, FILE **fp);
 isc_result_t	  removetmpkey(isc_mem_t *mctx, const char *file);
 void		  clean_trustedkey(void);
-void		  insert_trustedkey(dst_key_t  * key);
+void		  insert_trustedkey(dst_key_t * key);
 #if DIG_SIGCHASE_BU
 isc_result_t	  getneededrr(dns_message_t *msg);
 void		  sigchase_bottom_up(dns_message_t *msg);
@@ -1135,14 +1135,13 @@
 		goto failure;
 	}
 	result = dns_tsigkey_createfromkey(dst_key_name(dstkey), hmacname,
-					   dstkey, ISC_FALSE, NULL, 0, 0,
+					   &dstkey, ISC_FALSE, NULL, 0, 0,
 					   mctx, NULL, &key);
 	if (result != ISC_R_SUCCESS) {
 		printf(";; Couldn't create key %s: %s\n",
 		       keynametext, isc_result_totext(result));
 		goto failure;
 	}
-	dstkey = NULL;
  failure:
 	if (dstkey != NULL)
 		dst_key_free(&dstkey);
@@ -4053,14 +4052,15 @@
 }
 
 void
-insert_trustedkey(dst_key_t * key)
+insert_trustedkey(dst_key_t **keyp)
 {
-	if (key == NULL)
+	if (*keyp == NULL)
 		return;
 	if (tk_list.nb_tk >= MAX_TRUSTED_KEY)
 		return;
 
-	tk_list.key[tk_list.nb_tk++] = key;
+	tk_list.key[tk_list.nb_tk++] = *keyp;
+	*keyp = NULL;
 	return;
 }
 
@@ -4234,11 +4234,12 @@
 			fclose(fp);
 			return (ISC_R_FAILURE);
 		}
-		insert_trustedkey(key);
 #if 0
 		dst_key_tofile(key, DST_TYPE_PUBLIC,"/tmp");
 #endif
-		key = NULL;
+		insert_trustedkey(&key);
+		if (key != NULL)
+			dst_key_free(&key);
 	}
 	return (ISC_R_SUCCESS);
 }
Index: bind9/bin/named/server.c
diff -u bind9/bin/named/server.c:1.586 bind9/bin/named/server.c:1.586.8.1
--- bind9/bin/named/server.c:1.586	Tue Nov 16 01:37:36 2010
+++ bind9/bin/named/server.c	Wed Nov 24 07:26:28 2010
@@ -634,6 +634,8 @@
 	}
 
  cleanup:
+	if (dstkey != NULL)
+		dst_key_free(&dstkey);
 	if (secroots != NULL)
 		dns_keytable_detach(&secroots);
 	if (result == DST_R_NOCRYPTO)
@@ -3565,10 +3567,9 @@
 
 	/* Store the key in tsigkey. */
 	isc_stdtime_get(&now);
-	CHECK(dns_tsigkey_createfromkey(dst_key_name(key), algname, key,
+	CHECK(dns_tsigkey_createfromkey(dst_key_name(key), algname, &key,
 					ISC_FALSE, NULL, now, now, mctx, NULL,
 					&tsigkey));
-	key = NULL;		/* ownership of key has been transferred */
 
 	/* Dump the key to the key file. */
 	fp = ns_os_openfile(filename, S_IRUSR|S_IWUSR, ISC_TRUE);
Index: bind9/bin/nsupdate/nsupdate.c
diff -u bind9/bin/nsupdate/nsupdate.c:1.182 bind9/bin/nsupdate/nsupdate.c:1.182.42.1
--- bind9/bin/nsupdate/nsupdate.c:1.182	Tue Aug 10 23:48:19 2010
+++ bind9/bin/nsupdate/nsupdate.c	Wed Nov 24 07:26:28 2010
@@ -682,7 +682,7 @@
 	}
 	if (hmacname != NULL) {
 		result = dns_tsigkey_createfromkey(dst_key_name(dstkey),
-						   hmacname, dstkey, ISC_FALSE,
+						   hmacname, &dstkey, ISC_FALSE,
 						   NULL, 0, 0, mctx, NULL,
 						   &tsigkey);
 		if (result != ISC_R_SUCCESS) {
Index: bind9/bin/tests/system/tkey/keydelete.c
diff -u bind9/bin/tests/system/tkey/keydelete.c:1.13 bind9/bin/tests/system/tkey/keydelete.c:1.13.332.1
--- bind9/bin/tests/system/tkey/keydelete.c:1.13	Sun Jul 19 23:47:55 2009
+++ bind9/bin/tests/system/tkey/keydelete.c	Wed Nov 24 07:26:28 2010
@@ -230,7 +230,7 @@
 	CHECK("dst_key_fromnamedfile", result);
 	result = dns_tsigkey_createfromkey(dst_key_name(dstkey),
 					   DNS_TSIG_HMACMD5_NAME,
-					   dstkey, ISC_TRUE, NULL, 0, 0,
+					   &dstkey, ISC_TRUE, NULL, 0, 0,
 					   mctx, ring, &tsigkey);
 	CHECK("dns_tsigkey_createfromkey", result);
 
Index: bind9/lib/dns/client.c
diff -u bind9/lib/dns/client.c:1.10 bind9/lib/dns/client.c:1.10.92.1
--- bind9/lib/dns/client.c:1.10	Wed May 19 07:09:25 2010
+++ bind9/lib/dns/client.c	Wed Nov 24 07:26:28 2010
@@ -1424,6 +1424,8 @@
 	result = dns_keytable_add(secroots, ISC_FALSE, &dstkey);
 
  cleanup:
+	if (dstkey != NULL)
+		dns_key_free(&dstkey);
 	if (view != NULL)
 		dns_view_detach(&view);
 	if (secroots != NULL)
Index: bind9/lib/dns/dst_api.c
diff -u bind9/lib/dns/dst_api.c:1.51 bind9/lib/dns/dst_api.c:1.51.92.1
--- bind9/lib/dns/dst_api.c:1.51	Thu May 13 03:08:30 2010
+++ bind9/lib/dns/dst_api.c	Wed Nov 24 07:26:28 2010
@@ -544,6 +544,7 @@
 
 	*keyp = key;
 	return (ISC_R_SUCCESS);
+
  out:
 	if (pubkey != NULL)
 		dst_key_free(&pubkey);
Index: bind9/lib/dns/tkey.c
diff -u bind9/lib/dns/tkey.c:1.94 bind9/lib/dns/tkey.c:1.94.54.1
--- bind9/lib/dns/tkey.c:1.94	Fri Jul  9 23:46:51 2010
+++ bind9/lib/dns/tkey.c	Wed Nov 24 07:26:28 2010
@@ -495,7 +495,7 @@
 			expire = now + lifetime;
 #endif
 		RETERR(dns_tsigkey_createfromkey(name, &tkeyin->algorithm,
-						 dstkey, ISC_TRUE,
+						 &dstkey, ISC_TRUE,
 						 dns_fixedname_name(&principal),
 						 now, expire, ring->mctx, ring,
 						 NULL));
@@ -1280,15 +1280,13 @@
 	isc_buffer_init(&intoken, rtkey.key, rtkey.keylen);
 	RETERR(dst_gssapi_initctx(gname, &intoken, outtoken, context));
 
-	dstkey = NULL;
 	RETERR(dst_key_fromgssapi(dns_rootname, *context, rmsg->mctx,
 				  &dstkey));
 
 	RETERR(dns_tsigkey_createfromkey(tkeyname, DNS_TSIG_GSSAPI_NAME,
-					 dstkey, ISC_FALSE, NULL,
+					 &dstkey, ISC_FALSE, NULL,
 					 rtkey.inception, rtkey.expire,
 					 ring->mctx, ring, outkey));
-
 	dns_rdata_freestruct(&rtkey);
 	return (result);
 
@@ -1296,6 +1294,8 @@
 	/*
 	 * XXXSRA This probably leaks memory from rtkey and qtkey.
 	 */
+	if (dstkey != NULL)
+		dst_key_free(&dstkey);
 	return (result);
 }
 
@@ -1406,7 +1406,6 @@
 	if (result != DNS_R_CONTINUE && result != ISC_R_SUCCESS)
 		return (result);
 
-	dstkey = NULL;
 	RETERR(dst_key_fromgssapi(dns_rootname, *context, rmsg->mctx,
 				  &dstkey));
 
@@ -1420,10 +1419,9 @@
 					 (win2k
 					  ? DNS_TSIG_GSSAPIMS_NAME
 					  : DNS_TSIG_GSSAPI_NAME),
-					 dstkey, ISC_TRUE, NULL,
+					 &dstkey, ISC_TRUE, NULL,
 					 rtkey.inception, rtkey.expire,
 					 ring->mctx, ring, outkey));
-
 	dns_rdata_freestruct(&rtkey);
 	return (result);
 
@@ -1432,5 +1430,7 @@
 	 * XXXSRA This probably leaks memory from qtkey.
 	 */
 	dns_rdata_freestruct(&rtkey);
+	if (dstkey != NULL)
+		dst_key_free(&dstkey);
 	return (result);
 }
Index: bind9/lib/dns/tsec.c
diff -u bind9/lib/dns/tsec.c:1.4 bind9/lib/dns/tsec.c:1.4.310.1
--- bind9/lib/dns/tsec.c:1.4	Wed Sep  2 23:48:02 2009
+++ bind9/lib/dns/tsec.c	Wed Nov 24 07:26:28 2010
@@ -44,14 +44,16 @@
 };
 
 isc_result_t
-dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t *key,
+dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp,
 		dns_tsec_t **tsecp)
 {
 	isc_result_t result;
 	dns_tsec_t *tsec;
 	dns_tsigkey_t *tsigkey = NULL;
 	dns_name_t *algname;
+	dst_key_t *key;
 
+	REQUIRE(keyp != NULL && *keyp != NULL);
 	REQUIRE(mctx != NULL);
 	REQUIRE(tsecp != NULL && *tsecp == NULL);
 
@@ -59,6 +61,8 @@
 	if (tsec == NULL)
 		return (ISC_R_NOMEMORY);
 
+	key = *keyp;
+
 	tsec->type = type;
 	tsec->mctx = mctx;
 
@@ -88,7 +92,7 @@
 			return (DNS_R_BADALG);
 		}
 		result = dns_tsigkey_createfromkey(dst_key_name(key),
-						   algname, key, ISC_FALSE,
+						   algname, keyp, ISC_FALSE,
 						   NULL, 0, 0, mctx, NULL,
 						   &tsigkey);
 		if (result != ISC_R_SUCCESS) {
@@ -99,6 +103,7 @@
 		break;
 	case dns_tsectype_sig0:
 		tsec->ukey.key = key;
+		*keyp = NULL;
 		break;
 	default:
 		INSIST(0);
@@ -107,7 +112,7 @@
 	tsec->magic = DNS_TSEC_MAGIC;
 
 	*tsecp = tsec;
-
+	ENSURE(*keyp == NULL);
 	return (ISC_R_SUCCESS);
 }
 
Index: bind9/lib/dns/tsig.c
diff -u bind9/lib/dns/tsig.c:1.141 bind9/lib/dns/tsig.c:1.141.56.1
--- bind9/lib/dns/tsig.c:1.141	Fri Jul  9 05:13:15 2010
+++ bind9/lib/dns/tsig.c	Wed Nov 24 07:26:28 2010
@@ -287,7 +287,7 @@
 
 isc_result_t
 dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
-			  dst_key_t *dstkey, isc_boolean_t generated,
+			  dst_key_t **dstkeyp, isc_boolean_t generated,
 			  dns_name_t *creator, isc_stdtime_t inception,
 			  isc_stdtime_t expire, isc_mem_t *mctx,
 			  dns_tsig_keyring_t *ring, dns_tsigkey_t **key)
@@ -295,6 +295,7 @@
 	dns_tsigkey_t *tkey;
 	isc_result_t ret;
 	unsigned int refs = 0;
+	dst_key_t *dstkey;
 
 	REQUIRE(key == NULL || *key == NULL);
 	REQUIRE(name != NULL);
@@ -302,6 +303,10 @@
 	REQUIRE(mctx != NULL);
 	REQUIRE(key != NULL || ring != NULL);
 
+	if (dstkeyp != NULL)
+		dstkey = *dstkeyp;
+	else
+		dstkey = NULL;
 	tkey = (dns_tsigkey_t *) isc_mem_get(mctx, sizeof(dns_tsigkey_t));
 	if (tkey == NULL)
 		return (ISC_R_NOMEMORY);
@@ -436,6 +441,8 @@
 			      namestr);
 	}
 
+	if (dstkeyp != NULL)
+		*dstkeyp = NULL;
 	if (key != NULL)
 		*key = tkey;
 
@@ -623,7 +630,7 @@
 	} else if (length > 0)
 		return (DNS_R_BADALG);
 
-	result = dns_tsigkey_createfromkey(name, algorithm, dstkey,
+	result = dns_tsigkey_createfromkey(name, algorithm, &dstkey,
 					   generated, creator,
 					   inception, expire, mctx, ring, key);
 	if (result != ISC_R_SUCCESS && dstkey != NULL)
Index: bind9/lib/dns/zone.c
diff -u bind9/lib/dns/zone.c:1.574 bind9/lib/dns/zone.c:1.574.36.1
--- bind9/lib/dns/zone.c:1.574	Mon Sep  6 04:41:13 2010
+++ bind9/lib/dns/zone.c	Wed Nov 24 07:26:29 2010
@@ -2831,6 +2831,7 @@
 	isc_buffer_t buffer;
 	dns_view_t *view;
 	dns_keytable_t *sr = NULL;
+	dst_key_t *dstkey = NULL;
 
 	/* Convert dnskey to DST key. */
 	isc_buffer_init(&buffer, data, sizeof(data));
@@ -2839,18 +2840,19 @@
 
 	for (view = ISC_LIST_HEAD(*viewlist); view != NULL;
 	     view = ISC_LIST_NEXT(view, link)) {
-		dst_key_t *key = NULL;
 
 		result = dns_view_getsecroots(view, &sr);
 		if (result != ISC_R_SUCCESS)
 			continue;
 
-		CHECK(dns_dnssec_keyfromrdata(keyname, &rdata, mctx, &key));
-		CHECK(dns_keytable_add(sr, ISC_TRUE, &key));
+		CHECK(dns_dnssec_keyfromrdata(keyname, &rdata, mctx, &dstkey));
+		CHECK(dns_keytable_add(sr, ISC_TRUE, &dstkey));
 		dns_keytable_detach(&sr);
 	}
 
   failure:
+	if (dstkey != NULL)
+		dst_key_free(&dstkey);
 	if (sr != NULL)
 		dns_keytable_detach(&sr);
 	return;
@@ -3235,6 +3237,7 @@
 			dns_fixedname_t fname;
 			dns_name_t *keyname;
 			dst_key_t *key;
+
 			key = dns_keynode_key(keynode);
 			dns_fixedname_init(&fname);
 
@@ -4450,6 +4453,7 @@
 	isc_result_t result;
 	dns_dbnode_t *node = NULL;
 	const char *directory = dns_zone_getkeydirectory(zone);
+
 	CHECK(dns_db_findnode(db, dns_db_origin(db), ISC_FALSE, &node));
 	result = dns_dnssec_findzonekeys2(db, ver, node, dns_db_origin(db),
 					  directory, mctx, maxkeys, keys,
Index: bind9/lib/dns/include/dns/tsec.h
diff -u bind9/lib/dns/include/dns/tsec.h:1.3 bind9/lib/dns/include/dns/tsec.h:1.3.310.1
--- bind9/lib/dns/include/dns/tsec.h:1.3	Wed Sep  2 23:48:02 2009
+++ bind9/lib/dns/include/dns/tsec.h	Wed Nov 24 07:26:29 2010
@@ -65,7 +65,7 @@
 } dns_tsectype_t;
 
 isc_result_t
-dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t *key,
+dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp,
 		dns_tsec_t **tsecp);
 /*%<
  * Create a TSEC structure and stores a type-dependent key structure in it.
Index: bind9/lib/dns/include/dns/tsig.h
diff -u bind9/lib/dns/include/dns/tsig.h:1.55 bind9/lib/dns/include/dns/tsig.h:1.55.54.1
--- bind9/lib/dns/include/dns/tsig.h:1.55	Fri Jul  9 23:46:51 2010
+++ bind9/lib/dns/include/dns/tsig.h	Wed Nov 24 07:26:29 2010
@@ -103,7 +103,7 @@
 
 isc_result_t
 dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
-			  dst_key_t *dstkey, isc_boolean_t generated,
+			  dst_key_t **dstkeyp, isc_boolean_t generated,
 			  dns_name_t *creator, isc_stdtime_t inception,
 			  isc_stdtime_t expire, isc_mem_t *mctx,
 			  dns_tsig_keyring_t *ring, dns_tsigkey_t **key);
Index: bind9/lib/export/samples/sample-update.c
diff -u bind9/lib/export/samples/sample-update.c:1.5 bind9/lib/export/samples/sample-update.c:1.5.272.1
--- bind9/lib/export/samples/sample-update.c:1.5	Tue Sep 29 15:06:07 2009
+++ bind9/lib/export/samples/sample-update.c	Wed Nov 24 07:26:29 2010
@@ -747,6 +747,7 @@
 
 	result = dns_tsec_create(mctx, tsectype, dstkey, &tsec);
 	if (result != ISC_R_SUCCESS) {
+		dns_key_free(&dstkey);
 		fprintf(stderr, "could not create tsec: %s\n",
 			isc_result_totext(result));
 		exit(1);


More information about the bind-workers mailing list