No need to destroy filesources -- they will clean up when the source is destroyed

bin/dnssec/dnssectool.c

@@ -35,8 +35,6 @@
 extern int verbose;
 extern const char *program;
 
-static isc_entropysource_t *filesource = NULL;
-
 void
 fatal(const char *format, ...) {
 	va_list args;
@@ -171,17 +169,16 @@ setup_logging(int verbose, isc_mem_t *mctx, isc_log_t **logp) {
 void
 setup_entropy(isc_mem_t *mctx, const char *randomfile, isc_entropy_t **ectx) {
 	isc_result_t result;
+
 	result = isc_entropy_create(mctx, ectx);
 	if (result != ISC_R_SUCCESS)
 		fatal("could not create entropy object");
 	if (randomfile != NULL) {
-		result = isc_entropy_createfilesource(*ectx, randomfile, 0,
-						      &filesource);
+		result = isc_entropy_createfilesource(*ectx, randomfile, 0);
 		if (result == ISC_R_SUCCESS)
 			return;
 	}
-	result = isc_entropy_createfilesource(*ectx, "/dev/random", 0,
-					      &filesource);
+	result = isc_entropy_createfilesource(*ectx, "/dev/random", 0);
 	if (result != ISC_R_SUCCESS)
 		fatal("No randomfile specified, and /dev/random not present.");
 	return;
@@ -189,7 +186,5 @@ setup_entropy(isc_mem_t *mctx, const char *randomfile, isc_entropy_t **ectx) {
 
 void
 cleanup_entropy(isc_entropy_t **ectx) {
-	if (filesource != NULL)
-		isc_entropy_destroysource(&filesource);
 	isc_entropy_detach(ectx);
 }

bin/tests/dst/dst_test.c

@@ -229,7 +229,6 @@ int
 main(void) {
 	isc_mem_t *mctx = NULL;
 	isc_entropy_t *ectx = NULL;
-	isc_entropysource_t *devrandom = NULL, *randfile = NULL;
 	isc_buffer_t b;
 	dns_fixedname_t fname;
 	dns_name_t *name;
@@ -242,10 +241,8 @@ main(void) {
 	dns_result_register();
 
 	isc_entropy_create(mctx, &ectx);
-	isc_entropy_createfilesource(ectx, "/dev/random", 0,
-			&devrandom);
-	isc_entropy_createfilesource(ectx, "randomfile", 0,
-			&randfile);
+	isc_entropy_createfilesource(ectx, "/dev/random", 0);
+	isc_entropy_createfilesource(ectx, "randomfile", 0);
 	dst_lib_init(mctx, ectx, ISC_ENTROPY_BLOCKING|ISC_ENTROPY_GOODONLY);
 
 	dns_fixedname_init(&fname);
@@ -270,10 +267,6 @@ main(void) {
 	generate(DST_ALG_HMACMD5, mctx);
 
 	dst_lib_destroy();
-	if (devrandom != NULL)
-		isc_entropy_destroysource(&devrandom);
-	if (randfile != NULL)
-		isc_entropy_destroysource(&randfile);
 	isc_entropy_detach(&ectx);
 
 	isc_mem_put(mctx, current, 256);

bin/tests/dst/t_dst.c

@@ -338,8 +338,6 @@ static void
 t1(void) {
 	isc_mem_t	*mctx;
 	isc_entropy_t	*ectx;
-	isc_entropysource_t *devrandom;
-	isc_entropysource_t *randfile;
 	int		nfails;
 	int		nprobs;
 	int		result;
@@ -368,12 +366,8 @@ t1(void) {
 		t_result(T_UNRESOLVED);
 		return;
 	}
-	devrandom = NULL;
-	isc_entropy_createfilesource(ectx, "/dev/random", 0,
-			&devrandom);
-	randfile = NULL;
-	result = isc_entropy_createfilesource(ectx, "randomfile", 0,
-					      &randfile);
+	isc_entropy_createfilesource(ectx, "/dev/random", 0);
+	result = isc_entropy_createfilesource(ectx, "randomfile", 0);
 	if (isc_result != ISC_R_SUCCESS) {
 		t_info("isc_entropy_create failed %d\n",
 		       isc_result_totext(isc_result));
@@ -426,9 +420,6 @@ t1(void) {
 
 	dst_lib_destroy();
 
-	if (devrandom != NULL)
-		isc_entropy_destroysource(&devrandom);
-	isc_entropy_destroysource(&randfile);
 	isc_entropy_detach(&ectx);
 
 	isc_mem_destroy(&mctx);
@@ -813,8 +804,6 @@ t2_vfy(char **av) {
 	int		nprobs;
 	isc_mem_t	*mctx;
 	isc_entropy_t	*ectx;
-	isc_entropysource_t *devrandom;
-	isc_entropysource_t *randfile;
 	isc_result_t	isc_result;
 	int		result;
 
@@ -851,12 +840,8 @@ t2_vfy(char **av) {
 		       isc_result_totext(isc_result));
 		return(T_UNRESOLVED);
 	}
-	devrandom = NULL;
-	isc_entropy_createfilesource(ectx, "/dev/random", 0,
-			&devrandom);
-	randfile = NULL;
-	result = isc_entropy_createfilesource(ectx, "randomfile", 0,
-					      &randfile);
+	isc_entropy_createfilesource(ectx, "/dev/random", 0);
+	result = isc_entropy_createfilesource(ectx, "randomfile", 0);
 	if (isc_result != ISC_R_SUCCESS) {
 		t_info("isc_entropy_create failed %d\n",
 		       isc_result_totext(isc_result));
@@ -878,9 +863,6 @@ t2_vfy(char **av) {
 
 	dst_lib_destroy();
 
-	if (devrandom != NULL)
-		isc_entropy_destroysource(&devrandom);
-	isc_entropy_destroysource(&randfile);
 	isc_entropy_detach(&ectx);
 
 	isc_mem_destroy(&mctx);

bin/tests/entropy_test.c

@@ -53,7 +53,6 @@ main(int argc, char **argv) {
 	isc_mem_t *mctx;
 	unsigned char buffer[512];
 	isc_entropy_t *ent;
-	isc_entropysource_t *devrandom;
 	unsigned int returned;
 	unsigned int flags;
 	isc_result_t result;
@@ -72,17 +71,15 @@ main(int argc, char **argv) {
 	isc_entropy_stats(ent, stderr);
 
 #if 1
-	devrandom = NULL;
 	flags = 0;
-	CHECK("isc_entropy_createfilesource()",
-	      isc_entropy_createfilesource(ent, "/dev/random",
-					   flags, &devrandom));
+	CHECK("isc_entropy_createfilesource() 1",
+	      isc_entropy_createfilesource(ent, "/dev/random", flags));
+	CHECK("isc_entropy_createfilesource() 2",
+	      isc_entropy_createfilesource(ent, "/dev/random", flags));
 #else
-	devrandom = NULL;
 	flags = 0;
-	CHECK("isc_entropy_createfilesource()",
-	      isc_entropy_createfilesource(ent, "/tmp/foo",
-					   flags, &devrandom));
+	CHECK("isc_entropy_createfilesource() 3",
+	      isc_entropy_createfilesource(ent, "/tmp/foo", flags));
 #endif
 
 	fprintf(stderr,
@@ -100,7 +97,7 @@ main(int argc, char **argv) {
 
  any:
 	isc_entropy_stats(ent, stderr);
-	CHECK("isc_entropy_getdata()",
+	CHECK("isc_entropy_getdata() pseudorandom",
 	      isc_entropy_getdata(ent, buffer, 128, NULL, 0));
 	hex_dump("pseudorandom data", buffer, 128);
 
@@ -129,10 +126,10 @@ main(int argc, char **argv) {
 		isc_entropy_detach(&entcopy3);
 	}
 
-	isc_entropy_destroysource(&devrandom);
 	isc_entropy_detach(&ent);
 	isc_mem_stats(mctx, stderr);
 	isc_mem_destroy(&mctx);
 
 	return (0);
 }
+

bin/tests/tkey_test.c

@@ -359,7 +359,6 @@ main(int argc, char *argv[]) {
 	struct in_addr inaddr;
 	dns_fixedname_t fname;
 	dns_name_t *name;
-	isc_entropysource_t *devrandom;
 	isc_buffer_t b;
 	isc_result_t result;
 
@@ -373,10 +372,8 @@ main(int argc, char *argv[]) {
 	ectx = NULL;
 	RUNTIME_CHECK(isc_entropy_create(mctx, &ectx) == ISC_R_SUCCESS);
 
-	devrandom = NULL;
-	result = isc_entropy_createfilesource(ectx, "/dev/random", 0,
-					      &devrandom);
-	if (devrandom == NULL) {
+	result = isc_entropy_createfilesource(ectx, "/dev/random", 0);
+	if (result != ISC_R_SUCCESS) {
 		fprintf(stderr,
 			"%s only runs when /dev/random is available.\n",
 			argv[0]);
@@ -476,7 +473,6 @@ main(int argc, char *argv[]) {
 	isc_log_destroy(&log);
 
 	dst_lib_destroy();
-	isc_entropy_destroysource(&devrandom);
 	isc_entropy_detach(&ectx);
 
 	if (verbose)

lib/isc/include/isc/entropy.h

@@ -124,8 +124,7 @@ isc_entropy_detach(isc_entropy_t **entp);
 
 isc_result_t
 isc_entropy_createfilesource(isc_entropy_t *ent, const char *fname,
-			     unsigned int flags,
-			     isc_entropysource_t **sourcep);
+			     unsigned int flags);
 /*
  * Create a new entropy source from a file.
  *

lib/isc/unix/entropy.c

@@ -83,6 +83,8 @@ struct isc_entropy {
 	unsigned int			refcnt;
 	isc_uint32_t			initialized;
 	isc_entropypool_t		pool;
+	unsigned int			nsources;
+	isc_entropysource_t	       *nextsource;
 	ISC_LIST(isc_entropysource_t)	sources;
 };
 
@@ -163,22 +165,22 @@ subtract_entropy(isc_entropy_t *ent, isc_uint32_t entropy) {
  * very large.
  */
 static inline void
-add_pseudo(isc_entropy_t *ent, isc_uint32_t entropy) {
+add_pseudo(isc_entropy_t *ent, isc_uint32_t pseudo) {
 	/* clamp input.  Yes, this must be done. */
-	entropy = ISC_MIN(entropy, RND_POOLBITS * 8);
-	/* Add in the entropy we already have. */
-	entropy += ent->pool.pseudo;
+	pseudo = ISC_MIN(pseudo, RND_POOLBITS * 8);
+	/* Add in the pseudo we already have. */
+	pseudo += ent->pool.pseudo;
 	/* Clamp. */
-	ent->pool.pseudo = ISC_MIN(entropy, RND_POOLBITS * 8);
+	ent->pool.pseudo = ISC_MIN(pseudo, RND_POOLBITS * 8);
 }
 
 /*
- * Decrement the amount of entropy the pool has.
+ * Decrement the amount of pseudo the pool has.
  */
 static inline void
-subtract_pseudo(isc_entropy_t *ent, isc_uint32_t entropy) {
-	entropy = ISC_MIN(entropy, ent->pool.pseudo);
-	ent->pool.pseudo -= entropy;
+subtract_pseudo(isc_entropy_t *ent, isc_uint32_t pseudo) {
+	pseudo = ISC_MIN(pseudo, ent->pool.pseudo);
+	ent->pool.pseudo -= pseudo;
 }
 
 /*
@@ -320,6 +322,7 @@ static void
 fillpool(isc_entropy_t *ent, unsigned int needed, isc_boolean_t blocking) {
 	isc_uint32_t added;
 	isc_uint32_t remaining;
+	unsigned int nsource;
 	isc_entropysource_t *source;
 
 	REQUIRE(VALID_ENTROPY(ent));
@@ -372,7 +375,7 @@ fillpool(isc_entropy_t *ent, unsigned int needed, isc_boolean_t blocking) {
 	/*
 	 * But wait!  If we're not yet initialized, we need at least
 	 *	THRESHOLD_BITS
-	 * bytes of randomness.
+	 * of randomness.
 	 */
 	if (ent->initialized < THRESHOLD_BITS)
 		needed = ISC_MAX(needed, THRESHOLD_BITS - ent->initialized);
@@ -387,11 +390,19 @@ fillpool(isc_entropy_t *ent, unsigned int needed, isc_boolean_t blocking) {
 
 	added = 0;
 	remaining = needed;
+	if (ent->nextsource == NULL) {
+		ent->nextsource = ISC_LIST_HEAD(ent->sources);
+		if (ent->nextsource == NULL)
+			return;
+	}
+	source = ent->nextsource;
  again:
-	source = ISC_LIST_HEAD(ent->sources);
-	while (source != NULL && remaining > 0) {
+	for (nsource = 0 ; nsource < ent->nsources ; nsource++) {
 		isc_uint32_t got;
 
+		if (remaining == 0)
+			break;
+
 		got = 0;
 
 		if (source->type == ENTROPY_SOURCETYPE_FILE)
@@ -404,11 +415,14 @@ fillpool(isc_entropy_t *ent, unsigned int needed, isc_boolean_t blocking) {
 		else
 			remaining = 0;
 
+		source = ISC_LIST_NEXT(source, link);
+		if (source == NULL)
+			source = ISC_LIST_HEAD(ent->sources);
+
 		if (added >= needed)
 			break;
-
-		source = ISC_LIST_NEXT(source, link);
 	}
+	ent->nextsource = source;
 
 	if (blocking && remaining != 0) {
 		int fds;
@@ -652,6 +666,8 @@ isc_entropy_create(isc_mem_t *mctx, isc_entropy_t **entp) {
 	 * From here down, no failures will/can occur.
 	 */
 	ISC_LIST_INIT(ent->sources);
+	ent->nextsource = NULL;
+	ent->nsources = 0;
 	ent->mctx = NULL;
 	isc_mem_attach(mctx, &ent->mctx);
 	ent->refcnt = 1;
@@ -669,12 +685,63 @@ isc_entropy_create(isc_mem_t *mctx, isc_entropy_t **entp) {
 	return (ret);
 }
 
+/*
+ * Requires "ent" be locked.
+ */
+static void
+destroysource(isc_entropysource_t **sourcep) {
+	isc_entropysource_t *source;
+	isc_entropy_t *ent;
+	void *ptr;
+	int fd;
+
+	source = *sourcep;
+	*sourcep = NULL;
+	ent = source->ent;
+
+	ISC_LIST_UNLINK(ent->sources, source, link);
+	ent->nextsource = NULL;
+	REQUIRE(ent->nsources > 0);
+	ent->nsources--;
+
+	switch (source->type) {
+	case ENTROPY_SOURCETYPE_FILE:
+		fd = source->sources.file.fd;
+		if (fd >= 0)
+			close(fd);
+		break;
+	case ENTROPY_SOURCETYPE_SAMPLE:
+		ptr = source->sources.sample.samples;
+		if (ptr != NULL)
+			isc_mem_put(ent->mctx, ptr, RND_EVENTQSIZE * 4);
+		ptr = source->sources.sample.extra;
+		if (ptr != NULL)
+			isc_mem_put(ent->mctx, ptr, RND_EVENTQSIZE * 4);
+		break;
+	}
+
+	memset(source, 0, sizeof(isc_entropysource_t));
+
+	isc_mem_put(ent->mctx, source, sizeof(isc_entropysource_t));
+}
+
 static inline isc_boolean_t
 destroy_check(isc_entropy_t *ent) {
+	isc_entropysource_t *source;
+
 	if (ent->refcnt > 0)
 		return (ISC_FALSE);
-	if (!ISC_LIST_EMPTY(ent->sources))
+
+	source = ISC_LIST_HEAD(ent->sources);
+	while (source != NULL) {
+		switch (source->type) {
+		case ENTROPY_SOURCETYPE_FILE:
+			break;
+		default:
 			return (ISC_FALSE);
+		}
+		source = ISC_LIST_NEXT(source, link);
+	}
 
 	return (ISC_TRUE);
 }
@@ -682,18 +749,41 @@ destroy_check(isc_entropy_t *ent) {
 static void
 destroy(isc_entropy_t **entp) {
 	isc_entropy_t *ent;
+	isc_entropysource_t *source;
 	isc_mem_t *mctx;
 
 	REQUIRE(entp != NULL && *entp != NULL);
 	ent = *entp;
 	*entp = NULL;
 
+	LOCK(&ent->lock);
+
 	REQUIRE(ent->refcnt == 0);
+
+	/*
+	 * Here, detach non-sample sources.
+	 */
+	source = ISC_LIST_HEAD(ent->sources);
+	while (source != NULL) {
+		switch(source->type) {
+		case ENTROPY_SOURCETYPE_FILE:
+			destroysource(&source);
+			break;
+		}
+		source = ISC_LIST_HEAD(ent->sources);
+	}
+
+	/*
+	 * If there are other types of sources, we've found a bug.
+	 */
 	REQUIRE(ISC_LIST_EMPTY(ent->sources));
 
 	mctx = ent->mctx;
 
 	isc_entropypool_invalidate(&ent->pool);
+
+	UNLOCK(&ent->lock);
+
 	isc_mutex_destroy(&ent->lock);
 
 	memset(ent, 0, sizeof(isc_entropy_t));
@@ -726,8 +816,7 @@ make_nonblock(int fd) {
 
 isc_result_t
 isc_entropy_createfilesource(isc_entropy_t *ent, const char *fname,
-			     unsigned int flags,
-			     isc_entropysource_t **sourcep)
+			     unsigned int flags)
 {
 	int fd;
 	isc_result_t ret;
@@ -735,7 +824,6 @@ isc_entropy_createfilesource(isc_entropy_t *ent, const char *fname,
 
 	REQUIRE(VALID_ENTROPY(ent));
 	REQUIRE(fname != NULL);
-	REQUIRE(sourcep != NULL && *sourcep == NULL);
 
 	LOCK(&ent->lock);
 
@@ -777,8 +865,7 @@ isc_entropy_createfilesource(isc_entropy_t *ent, const char *fname,
 	 * Hook it into the entropy system.
 	 */
 	ISC_LIST_APPEND(ent->sources, source, link);
-
-	*sourcep = source;
+	ent->nsources++;
 
 	UNLOCK(&ent->lock);
 	return (ISC_R_SUCCESS);
@@ -798,8 +885,6 @@ void
 isc_entropy_destroysource(isc_entropysource_t **sourcep) {
 	isc_entropysource_t *source;
 	isc_entropy_t *ent;
-	void *ptr;
-	int fd;
 	isc_boolean_t killit;
 
 	REQUIRE(sourcep != NULL);
@@ -813,27 +898,7 @@ isc_entropy_destroysource(isc_entropysource_t **sourcep) {
 
 	LOCK(&ent->lock);
 
-	ISC_LIST_UNLINK(ent->sources, source, link);
-
-	switch (source->type) {
-	case ENTROPY_SOURCETYPE_FILE:
-		fd = source->sources.file.fd;
-		if (fd >= 0)
-			close(fd);
-		break;
-	case ENTROPY_SOURCETYPE_SAMPLE:
-		ptr = source->sources.sample.samples;
-		if (ptr != NULL)
-			isc_mem_put(ent->mctx, ptr, RND_EVENTQSIZE * 4);
-		ptr = source->sources.sample.extra;
-		if (ptr != NULL)
-			isc_mem_put(ent->mctx, ptr, RND_EVENTQSIZE * 4);
-		break;
-	}
-
-	memset(source, 0, sizeof(isc_entropysource_t));
-
-	isc_mem_put(ent->mctx, source, sizeof(isc_entropysource_t));
+	destroysource(&source);
 
 	killit = destroy_check(ent);
 
@@ -852,6 +917,8 @@ isc_entropy_createsamplesource(isc_entropy_t *ent,
 
 	LOCK(&ent->lock);
 
+	ent->nsources++;
+
 	UNLOCK(&ent->lock);
 
 	return (ISC_R_NOTIMPLEMENTED);
@@ -888,10 +955,12 @@ isc_entropy_stats(isc_entropy_t *ent, FILE *out) {
 
 	fprintf(out,
 		"Entropy pool %p:  refcnt %u"
-		" cursor %u, rotate %u entropy %u pseudo %u\n",
+		" cursor %u, rotate %u entropy %u pseudo %u nsources %u"
+		" nextsource %p initialized %u\n",
 		ent, ent->refcnt,
 		ent->pool.cursor, ent->pool.rotate,
-		ent->pool.entropy, ent->pool.pseudo);
+		ent->pool.entropy, ent->pool.pseudo,
+		ent->nsources, ent->nextsource, ent->initialized);
 }
 
 void