[PATCH 0/4] DRBG: Fixes for sparse tool reports

[PATCH 0/4] DRBG: Fixes for sparse tool reports

[Stephan Mueller]

Hi,

The following patches cover requested changes based on the sparse tool test
run and suggestions by peer reviewers.

In addition, a patch to make the CTR DRBG more efficient is added.

Stephan Mueller (4):
  DRBG: use of kernel linked list
  DRBG: cleanup of preprocessor macros
  DRBG: Fix format string for debugging statements
  DRBG: Call CTR DRBG DF function only once

 crypto/drbg.c         | 302 ++++++++++++++++++++++++++++----------------------
 include/crypto/drbg.h |   7 +-
 2 files changed, 174 insertions(+), 135 deletions(-)

-- 
1.9.3

[PATCH 1/4] DRBG: use of kernel linked list

[Stephan Mueller]

The DRBG-style linked list to manage input data that is fed into the
cipher invocations is replaced with the kernel linked list
implementation.

The change is transparent to users of the interfaces offered by the
DRBG. Therefore, no changes to the testmgr code is needed.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c         | 233 +++++++++++++++++++++++++++-----------------------
 include/crypto/drbg.h |   7 +-
 2 files changed, 128 insertions(+), 112 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 99fa8f8..6679a26 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -370,13 +370,12 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg);
 /* BCC function for CTR DRBG as defined in 10.4.3 */
 static int drbg_ctr_bcc(struct drbg_state *drbg,
 			unsigned char *out, const unsigned char *key,
-			struct drbg_string *in)
+			struct list_head *in)
 {
-	int ret = -EFAULT;
-	struct drbg_string *curr = in;
-	size_t inpos = curr->len;
-	const unsigned char *pos = curr->buf;
+	int ret = 0;
+	struct drbg_string *curr = NULL;
 	struct drbg_string data;
+	short cnt = 0;
 
 	drbg_string_fill(&data, out, drbg_blocklen(drbg));
 
@@ -384,39 +383,29 @@ static int drbg_ctr_bcc(struct drbg_state *drbg,
 	memset(out, 0, drbg_blocklen(drbg));
 
 	/* 10.4.3 step 2 / 4 */
-	while (inpos) {
-		short cnt = 0;
+	list_for_each_entry(curr, in, list) {
+		const unsigned char *pos = curr->buf;
+		size_t len = curr->len;
 		/* 10.4.3 step 4.1 */
-		for (cnt = 0; cnt < drbg_blocklen(drbg); cnt++) {
-			out[cnt] ^= *pos;
-			pos++; inpos--;
-			/*
-			 * The following branch implements the linked list
-			 * iteration of drbg_string *in. If we are at the
-			 * end of the current list member, we have to start
-			 * using the next member if available. The inpos
-			 * value always points to the current byte and will
-			 * be zero if we have processed the last byte of
-			 * the last linked list member.
-			 */
-			if (0 == inpos) {
-				curr = curr->next;
-				if (NULL != curr) {
-					pos = curr->buf;
-					inpos = curr->len;
-				} else {
-					inpos = 0;
-					break;
-				}
+		while (len) {
+			/* 10.4.3 step 4.2 */
+			if (drbg_blocklen(drbg) == cnt) {
+				cnt = 0;
+				ret = drbg_kcapi_sym(drbg, key, out, &data);
+				if (ret)
+					return ret;
 			}
+			out[cnt] ^= *pos;
+			pos++;
+			cnt++;
+			len--;
 		}
-		/* 10.4.3 step 4.2 */
-		ret = drbg_kcapi_sym(drbg, key, out, &data);
-		if (ret)
-			return ret;
-		/* 10.4.3 step 2 */
 	}
-	return 0;
+	/* 10.4.3 step 4.2 for last block */
+	if (cnt)
+		ret = drbg_kcapi_sym(drbg, key, out, &data);
+
+	return ret;
 }
 
 /*
@@ -453,13 +442,13 @@ static int drbg_ctr_bcc(struct drbg_state *drbg,
 /* Derivation Function for CTR DRBG as defined in 10.4.2 */
 static int drbg_ctr_df(struct drbg_state *drbg,
 		       unsigned char *df_data, size_t bytes_to_return,
-		       struct drbg_string *addtl)
+		       struct list_head *seedlist)
 {
 	int ret = -EFAULT;
 	unsigned char L_N[8];
 	/* S3 is input */
 	struct drbg_string S1, S2, S4, cipherin;
-	struct drbg_string *tempstr = addtl;
+	LIST_HEAD(bcc_list);
 	unsigned char *pad = df_data + drbg_statelen(drbg);
 	unsigned char *iv = pad + drbg_blocklen(drbg);
 	unsigned char *temp = iv + drbg_blocklen(drbg);
@@ -476,6 +465,7 @@ static int drbg_ctr_df(struct drbg_state *drbg,
 	unsigned char *X;
 	size_t generated_len = 0;
 	size_t inputlen = 0;
+	struct drbg_string *seed = NULL;
 
 	memset(pad, 0, drbg_blocklen(drbg));
 	memset(iv, 0, drbg_blocklen(drbg));
@@ -488,8 +478,8 @@ static int drbg_ctr_df(struct drbg_state *drbg,
 		return -EINVAL;
 
 	/* 10.4.2 step 2 -- calculate the entire length of all input data */
-	for (; NULL != tempstr; tempstr = tempstr->next)
-		inputlen += tempstr->len;
+	list_for_each_entry(seed, seedlist, list)
+		inputlen += seed->len;
 	drbg_int2byte(&L_N[0], inputlen, 4);
 
 	/* 10.4.2 step 3 */
@@ -510,20 +500,12 @@ static int drbg_ctr_df(struct drbg_state *drbg,
 
 	/* 10.4.2 step 4 -- first fill the linked list and then order it */
 	drbg_string_fill(&S1, iv, drbg_blocklen(drbg));
+	list_add_tail(&S1.list, &bcc_list);
 	drbg_string_fill(&S2, L_N, sizeof(L_N));
+	list_add_tail(&S2.list, &bcc_list);
+	list_splice_tail(seedlist, &bcc_list);
 	drbg_string_fill(&S4, pad, padlen);
-	S1.next = &S2;
-	S2.next = addtl;
-
-	/*
-	 * Splice in addtl between S2 and S4 -- we place S4 at the end
-	 * of the input data chain. As this code is only triggered when
-	 * addtl is not NULL, no NULL checks are necessary.
-	 */
-	tempstr = addtl;
-	while (tempstr->next)
-		tempstr = tempstr->next;
-	tempstr->next = &S4;
+	list_add_tail(&S4.list, &bcc_list);
 
 	/* 10.4.2 step 9 */
 	while (templen < (drbg_keylen(drbg) + (drbg_blocklen(drbg)))) {
@@ -534,7 +516,7 @@ static int drbg_ctr_df(struct drbg_state *drbg,
 		 */
 		drbg_int2byte(iv, i, 4);
 		/* 10.4.2 step 9.2 -- BCC and concatenation with temp */
-		ret = drbg_ctr_bcc(drbg, temp + templen, K, &S1);
+		ret = drbg_ctr_bcc(drbg, temp + templen, K, &bcc_list);
 		if (ret)
 			goto out;
 		/* 10.4.2 step 9.3 */
@@ -578,8 +560,8 @@ out:
 }
 
 /* update function of CTR DRBG as defined in 10.2.1.2 */
-static int drbg_ctr_update(struct drbg_state *drbg,
-			   struct drbg_string *addtl, int reseed)
+static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
+			   int reseed)
 {
 	int ret = -EFAULT;
 	/* 10.2.1.2 step 1 */
@@ -595,9 +577,8 @@ static int drbg_ctr_update(struct drbg_state *drbg,
 	memset(df_data, 0, drbg_statelen(drbg));
 
 	/* 10.2.1.3.2 step 2 and 10.2.1.4.2 step 2 */
-	if (addtl && 0 < addtl->len) {
-		ret = drbg_ctr_df(drbg, df_data, drbg_statelen(drbg),
-				  addtl);
+	if (seed) {
+		ret = drbg_ctr_df(drbg, df_data, drbg_statelen(drbg), seed);
 		if (ret)
 			goto out;
 	}
@@ -657,8 +638,10 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 
 	/* 10.2.1.5.2 step 2 */
 	if (addtl && 0 < addtl->len) {
-		addtl->next = NULL;
-		ret = drbg_ctr_update(drbg, addtl, 1);
+		LIST_HEAD(addtllist);
+
+		list_add_tail(&addtl->list, &addtllist);
+		ret = drbg_ctr_update(drbg, &addtllist, 1);
 		if (ret)
 			return 0;
 	}
@@ -689,16 +672,21 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 			drbg_add_buf(drbg->V, drbg_blocklen(drbg), &prefix, 1);
 	}
 
-	/* 10.2.1.5.2 step 6 */
-	if (addtl)
-		addtl->next = NULL;
 	/*
+	 * 10.2.1.5.2 step 6
 	 * The following call invokes the DF function again which could be
 	 * optimized. In step 2, the "additional_input" after step 2 is the
 	 * output of the DF function. If this result would be saved, the DF
 	 * function would not need to be invoked again at this point.
 	 */
-	ret = drbg_ctr_update(drbg, addtl, 1);
+	if (addtl && 0 < addtl->len) {
+		LIST_HEAD(addtllist);
+
+		list_add_tail(&addtl->list, &addtllist);
+		ret = drbg_ctr_update(drbg, &addtllist, 1);
+	} else {
+		ret = drbg_ctr_update(drbg, NULL, 1);
+	}
 	if (ret)
 		len = ret;
 
@@ -721,19 +709,21 @@ static struct drbg_state_ops drbg_ctr_ops = {
 
 #if defined(CONFIG_CRYPTO_DRBG_HASH) || defined(CONFIG_CRYPTO_DRBG_HMAC)
 static int drbg_kcapi_hash(struct drbg_state *drbg, const unsigned char *key,
-			   unsigned char *outval, const struct drbg_string *in);
+			   unsigned char *outval, const struct list_head *in);
 static int drbg_init_hash_kernel(struct drbg_state *drbg);
 static int drbg_fini_hash_kernel(struct drbg_state *drbg);
 #endif /* (CONFIG_CRYPTO_DRBG_HASH || CONFIG_CRYPTO_DRBG_HMAC) */
 
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
 /* update function of HMAC DRBG as defined in 10.1.2.2 */
-static int drbg_hmac_update(struct drbg_state *drbg,
-			    struct drbg_string *seed, int reseed)
+static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
+			    int reseed)
 {
 	int ret = -EFAULT;
 	int i = 0;
-	struct drbg_string seed1, seed2, cipherin;
+	struct drbg_string seed1, seed2, vdata;
+	LIST_HEAD(seedlist);
+	LIST_HEAD(vdatalist);
 
 	if (!reseed) {
 		/* 10.1.2.3 step 2 */
@@ -742,13 +732,16 @@ static int drbg_hmac_update(struct drbg_state *drbg,
 	}
 
 	drbg_string_fill(&seed1, drbg->V, drbg_statelen(drbg));
+	list_add_tail(&seed1.list, &seedlist);
 	/* buffer of seed2 will be filled in for loop below with one byte */
 	drbg_string_fill(&seed2, NULL, 1);
-	seed1.next = &seed2;
+	list_add_tail(&seed2.list, &seedlist);
 	/* input data of seed is allowed to be NULL at this point */
-	seed2.next = seed;
+	if (seed)
+		list_splice_tail(seed, &seedlist);
 
-	drbg_string_fill(&cipherin, drbg->V, drbg_statelen(drbg));
+	drbg_string_fill(&vdata, drbg->V, drbg_statelen(drbg));
+	list_add_tail(&vdata.list, &vdatalist);
 	for (i = 2; 0 < i; i--) {
 		/* first round uses 0x0, second 0x1 */
 		unsigned char prefix = DRBG_PREFIX0;
@@ -756,17 +749,17 @@ static int drbg_hmac_update(struct drbg_state *drbg,
 			prefix = DRBG_PREFIX1;
 		/* 10.1.2.2 step 1 and 4 -- concatenation and HMAC for key */
 		seed2.buf = &prefix;
-		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->C, &seed1);
+		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->C, &seedlist);
 		if (ret)
 			return ret;
 
 		/* 10.1.2.2 step 2 and 5 -- HMAC for V */
-		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->V, &cipherin);
+		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->V, &vdatalist);
 		if (ret)
 			return ret;
 
 		/* 10.1.2.2 step 3 */
-		if (!seed || 0 == seed->len)
+		if (!seed)
 			return ret;
 	}
 
@@ -782,20 +775,24 @@ static int drbg_hmac_generate(struct drbg_state *drbg,
 	int len = 0;
 	int ret = 0;
 	struct drbg_string data;
+	LIST_HEAD(datalist);
 
 	/* 10.1.2.5 step 2 */
 	if (addtl && 0 < addtl->len) {
-		addtl->next = NULL;
-		ret = drbg_hmac_update(drbg, addtl, 1);
+		LIST_HEAD(addtllist);
+
+		list_add_tail(&addtl->list, &addtllist);
+		ret = drbg_hmac_update(drbg, &addtllist, 1);
 		if (ret)
 			return ret;
 	}
 
 	drbg_string_fill(&data, drbg->V, drbg_statelen(drbg));
+	list_add_tail(&data.list, &datalist);
 	while (len < buflen) {
 		unsigned int outlen = 0;
 		/* 10.1.2.5 step 4.1 */
-		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->V, &data);
+		ret = drbg_kcapi_hash(drbg, drbg->C, drbg->V, &datalist);
 		if (ret)
 			return ret;
 		outlen = (drbg_blocklen(drbg) < (buflen - len)) ?
@@ -809,9 +806,14 @@ static int drbg_hmac_generate(struct drbg_state *drbg,
 	}
 
 	/* 10.1.2.5 step 6 */
-	if (addtl)
-		addtl->next = NULL;
-	ret = drbg_hmac_update(drbg, addtl, 1);
+	if (addtl && 0 < addtl->len) {
+		LIST_HEAD(addtllist);
+
+		list_add_tail(&addtl->list, &addtllist);
+		ret = drbg_hmac_update(drbg, &addtllist, 1);
+	} else {
+		ret = drbg_hmac_update(drbg, NULL, 1);
+	}
 	if (ret)
 		return ret;
 
@@ -850,13 +852,13 @@ static struct drbg_state_ops drbg_hmac_ops = {
 /* Derivation Function for Hash DRBG as defined in 10.4.1 */
 static int drbg_hash_df(struct drbg_state *drbg,
 			unsigned char *outval, size_t outlen,
-			struct drbg_string *entropy)
+			struct list_head *entropylist)
 {
 	int ret = 0;
 	size_t len = 0;
 	unsigned char input[5];
 	unsigned char *tmp = drbg->scratchpad + drbg_statelen(drbg);
-	struct drbg_string data1;
+	struct drbg_string data;
 
 	memset(tmp, 0, drbg_blocklen(drbg));
 
@@ -865,14 +867,14 @@ static int drbg_hash_df(struct drbg_state *drbg,
 	drbg_int2byte(&input[1], (outlen * 8), 4);
 
 	/* 10.4.1 step 4.1 -- concatenation of data for input into hash */
-	drbg_string_fill(&data1, input, 5);
-	data1.next = entropy;
+	drbg_string_fill(&data, input, 5);
+	list_add(&data.list, entropylist);
 
 	/* 10.4.1 step 4 */
 	while (len < outlen) {
 		short blocklen = 0;
 		/* 10.4.1 step 4.1 */
-		ret = drbg_kcapi_hash(drbg, NULL, tmp, &data1);
+		ret = drbg_kcapi_hash(drbg, NULL, tmp, entropylist);
 		if (ret)
 			goto out;
 		/* 10.4.1 step 4.2 */
@@ -889,11 +891,13 @@ out:
 }
 
 /* update function for Hash DRBG as defined in 10.1.1.2 / 10.1.1.3 */
-static int drbg_hash_update(struct drbg_state *drbg, struct drbg_string *seed,
+static int drbg_hash_update(struct drbg_state *drbg, struct list_head *seed,
 			    int reseed)
 {
 	int ret = 0;
 	struct drbg_string data1, data2;
+	LIST_HEAD(datalist);
+	LIST_HEAD(datalist2);
 	unsigned char *V = drbg->scratchpad;
 	unsigned char prefix = DRBG_PREFIX1;
 
@@ -905,26 +909,25 @@ static int drbg_hash_update(struct drbg_state *drbg, struct drbg_string *seed,
 		/* 10.1.1.3 step 1 */
 		memcpy(V, drbg->V, drbg_statelen(drbg));
 		drbg_string_fill(&data1, &prefix, 1);
+		list_add_tail(&data1.list, &datalist);
 		drbg_string_fill(&data2, V, drbg_statelen(drbg));
-		data1.next = &data2;
-		data2.next = seed;
-	} else {
-		drbg_string_fill(&data1, seed->buf, seed->len);
-		data1.next = seed->next;
+		list_add_tail(&data2.list, &datalist);
 	}
+	list_splice_tail(seed, &datalist);
 
 	/* 10.1.1.2 / 10.1.1.3 step 2 and 3 */
-	ret = drbg_hash_df(drbg, drbg->V, drbg_statelen(drbg), &data1);
+	ret = drbg_hash_df(drbg, drbg->V, drbg_statelen(drbg), &datalist);
 	if (ret)
 		goto out;
 
 	/* 10.1.1.2 / 10.1.1.3 step 4  */
 	prefix = DRBG_PREFIX0;
 	drbg_string_fill(&data1, &prefix, 1);
+	list_add_tail(&data1.list, &datalist2);
 	drbg_string_fill(&data2, drbg->V, drbg_statelen(drbg));
-	data1.next = &data2;
+	list_add_tail(&data2.list, &datalist2);
 	/* 10.1.1.2 / 10.1.1.3 step 4 */
-	ret = drbg_hash_df(drbg, drbg->C, drbg_statelen(drbg), &data1);
+	ret = drbg_hash_df(drbg, drbg->C, drbg_statelen(drbg), &datalist2);
 
 out:
 	memset(drbg->scratchpad, 0, drbg_statelen(drbg));
@@ -937,7 +940,7 @@ static int drbg_hash_process_addtl(struct drbg_state *drbg,
 {
 	int ret = 0;
 	struct drbg_string data1, data2;
-	struct drbg_string *data3;
+	LIST_HEAD(datalist);
 	unsigned char prefix = DRBG_PREFIX2;
 
 	/* this is value w as per documentation */
@@ -950,11 +953,10 @@ static int drbg_hash_process_addtl(struct drbg_state *drbg,
 	/* 10.1.1.4 step 2a */
 	drbg_string_fill(&data1, &prefix, 1);
 	drbg_string_fill(&data2, drbg->V, drbg_statelen(drbg));
-	data3 = addtl;
-	data1.next = &data2;
-	data2.next = data3;
-	data3->next = NULL;
-	ret = drbg_kcapi_hash(drbg, NULL, drbg->scratchpad, &data1);
+	list_add_tail(&data1.list, &datalist);
+	list_add_tail(&data2.list, &datalist);
+	list_add_tail(&addtl->list, &datalist);
+	ret = drbg_kcapi_hash(drbg, NULL, drbg->scratchpad, &datalist);
 	if (ret)
 		goto out;
 
@@ -977,6 +979,7 @@ static int drbg_hash_hashgen(struct drbg_state *drbg,
 	unsigned char *src = drbg->scratchpad;
 	unsigned char *dst = drbg->scratchpad + drbg_statelen(drbg);
 	struct drbg_string data;
+	LIST_HEAD(datalist);
 	unsigned char prefix = DRBG_PREFIX1;
 
 	memset(src, 0, drbg_statelen(drbg));
@@ -986,10 +989,11 @@ static int drbg_hash_hashgen(struct drbg_state *drbg,
 	memcpy(src, drbg->V, drbg_statelen(drbg));
 
 	drbg_string_fill(&data, src, drbg_statelen(drbg));
+	list_add_tail(&data.list, &datalist);
 	while (len < buflen) {
 		unsigned int outlen = 0;
 		/* 10.1.1.4 step hashgen 4.1 */
-		ret = drbg_kcapi_hash(drbg, NULL, dst, &data);
+		ret = drbg_kcapi_hash(drbg, NULL, dst, &datalist);
 		if (ret) {
 			len = ret;
 			goto out;
@@ -1024,6 +1028,7 @@ static int drbg_hash_generate(struct drbg_state *drbg,
 	unsigned char req[8];
 	unsigned char prefix = DRBG_PREFIX3;
 	struct drbg_string data1, data2;
+	LIST_HEAD(datalist);
 
 	/* 10.1.1.4 step 2 */
 	ret = drbg_hash_process_addtl(drbg, addtl);
@@ -1036,9 +1041,10 @@ static int drbg_hash_generate(struct drbg_state *drbg,
 	memset(drbg->scratchpad, 0, drbg_blocklen(drbg));
 	/* 10.1.1.4 step 4 */
 	drbg_string_fill(&data1, &prefix, 1);
+	list_add_tail(&data1.list, &datalist);
 	drbg_string_fill(&data2, drbg->V, drbg_statelen(drbg));
-	data1.next = &data2;
-	ret = drbg_kcapi_hash(drbg, NULL, drbg->scratchpad, &data1);
+	list_add_tail(&data2.list, &datalist);
+	ret = drbg_kcapi_hash(drbg, NULL, drbg->scratchpad, &datalist);
 	if (ret) {
 		len = ret;
 		goto out;
@@ -1091,6 +1097,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 	unsigned char *entropy = NULL;
 	size_t entropylen = 0;
 	struct drbg_string data1;
+	LIST_HEAD(seedlist);
 
 	/* 9.1 / 9.2 / 9.3.1 step 3 */
 	if (pers && pers->len > (drbg_max_addtl(drbg))) {
@@ -1125,18 +1132,19 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 		get_random_bytes(entropy, entropylen);
 		drbg_string_fill(&data1, entropy, entropylen);
 	}
+	list_add_tail(&data1.list, &seedlist);
 
 	/*
 	 * concatenation of entropy with personalization str / addtl input)
 	 * the variable pers is directly handed in by the caller, so check its
 	 * contents whether it is appropriate
 	 */
-	if (pers && pers->buf && 0 < pers->len && NULL == pers->next) {
-		data1.next = pers;
+	if (pers && pers->buf && 0 < pers->len) {
+		list_add_tail(&pers->list, &seedlist);
 		pr_devel("DRBG: using personalization string\n");
 	}
 
-	ret = drbg->d_ops->update(drbg, &data1, reseed);
+	ret = drbg->d_ops->update(drbg, &seedlist, reseed);
 	if (ret)
 		goto out;
 
@@ -1634,15 +1642,16 @@ static int drbg_fini_hash_kernel(struct drbg_state *drbg)
 }
 
 static int drbg_kcapi_hash(struct drbg_state *drbg, const unsigned char *key,
-			   unsigned char *outval, const struct drbg_string *in)
+			   unsigned char *outval, const struct list_head *in)
 {
 	struct sdesc *sdesc = (struct sdesc *)drbg->priv_data;
+	struct drbg_string *input = NULL;
 
 	if (key)
 		crypto_shash_setkey(sdesc->shash.tfm, key, drbg_statelen(drbg));
 	crypto_shash_init(&sdesc->shash);
-	for (; NULL != in; in = in->next)
-		crypto_shash_update(&sdesc->shash, in->buf, in->len);
+	list_for_each_entry(input, in, list)
+		crypto_shash_update(&sdesc->shash, input->buf, input->len);
 	return crypto_shash_final(&sdesc->shash, outval);
 }
 #endif /* (CONFIG_CRYPTO_DRBG_HASH || CONFIG_CRYPTO_DRBG_HMAC) */
@@ -1777,12 +1786,15 @@ static int drbg_kcapi_random(struct crypto_rng *tfm, u8 *rdata,
 		return drbg_generate_long(drbg, rdata, dlen, NULL);
 	} else {
 		struct drbg_gen *data = (struct drbg_gen *)rdata;
+		struct drbg_string addtl;
 		/* catch NULL pointer */
 		if (!data)
 			return 0;
 		drbg_set_testdata(drbg, data->test_data);
+		/* linked list variable is now local to allow modification */
+		drbg_string_fill(&addtl, data->addtl->buf, data->addtl->len);
 		return drbg_generate_long(drbg, data->outbuf, data->outlen,
-					  data->addtl);
+					  &addtl);
 	}
 }
 
@@ -1812,7 +1824,10 @@ static int drbg_kcapi_reset(struct crypto_rng *tfm, u8 *seed, unsigned int slen)
 		if (!data)
 			return drbg_instantiate(drbg, NULL, coreref, pr);
 		drbg_set_testdata(drbg, data->test_data);
-		return drbg_instantiate(drbg, data->addtl, coreref, pr);
+		/* linked list variable is now local to allow modification */
+		drbg_string_fill(&seed_string, data->addtl->buf,
+				 data->addtl->len);
+		return drbg_instantiate(drbg, &seed_string, coreref, pr);
 	}
 }
 
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index b507c5b6..4065dfc 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -50,6 +50,7 @@
 #include <crypto/rng.h>
 #include <linux/fips.h>
 #include <linux/spinlock.h>
+#include <linux/list.h>
 
 /*
  * Concatenation Helper and string operation helper
@@ -64,7 +65,7 @@
 struct drbg_string {
 	const unsigned char *buf;
 	size_t len;
-	struct drbg_string *next;
+	struct list_head list;
 };
 
 static inline void drbg_string_fill(struct drbg_string *string,
@@ -72,7 +73,7 @@ static inline void drbg_string_fill(struct drbg_string *string,
 {
 	string->buf = buf;
 	string->len = len;
-	string->next = NULL;
+	INIT_LIST_HEAD(&string->list);
 }
 
 struct drbg_state;
@@ -97,7 +98,7 @@ struct drbg_core {
 };
 
 struct drbg_state_ops {
-	int (*update)(struct drbg_state *drbg, struct drbg_string *seed,
+	int (*update)(struct drbg_state *drbg, struct list_head *seed,
 		      int reseed);
 	int (*generate)(struct drbg_state *drbg,
 			unsigned char *buf, unsigned int buflen,
-- 
1.9.3

Re: [PATCH 1/4] DRBG: use of kernel linked list

[Herbert Xu]

On Sat, Jun 28, 2014 at 09:58:24PM +0200, Stephan Mueller wrote:
> The DRBG-style linked list to manage input data that is fed into the
> cipher invocations is replaced with the kernel linked list
> implementation.
> 
> The change is transparent to users of the interfaces offered by the
> DRBG. Therefore, no changes to the testmgr code is needed.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

Patch applied.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 2/4] DRBG: cleanup of preprocessor macros

[Stephan Mueller]

The structure used to construct the module description line was marked
problematic by the sparse code analysis tool. The module line
description now does not contain any ifdefs to prevent error reports
from sparse.

The preprocessor warning declaration was reported by sparse. It is
replaced in favor of an init function reporting the erroneous built of
the DRBG.

Lastly, a fix of the use use of CONFIG_CRYPTO_DRBG_HASH has been
applied.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 43 +++++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 6679a26..03a230e 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -102,8 +102,13 @@
 #if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
 	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
 	!defined(CONFIG_CRYPTO_DRBG_CTR)
-#warning "The DRBG code is useless without compiling at least one DRBG type"
-#endif
+#define CRYPTO_DRBG_NONE_STRING "none "
+static int __init drbg_init(void)
+{
+	pr_warn("DRBG: no DRBG core was compiled!\n");
+	return -EFAULT;
+}
+#else
 
 /***************************************************************
  * Backend cipher definitions available to DRBG
@@ -362,6 +367,7 @@ static inline void drbg_add_buf(unsigned char *dst, size_t dstlen,
  ******************************************************************/
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
+#define CRYPTO_DRBG_CTR_STRING "CTR "
 static int drbg_kcapi_sym(struct drbg_state *drbg, const unsigned char *key,
 			  unsigned char *outval, const struct drbg_string *in);
 static int drbg_init_sym_kernel(struct drbg_state *drbg);
@@ -715,6 +721,7 @@ static int drbg_fini_hash_kernel(struct drbg_state *drbg);
 #endif /* (CONFIG_CRYPTO_DRBG_HASH || CONFIG_CRYPTO_DRBG_HMAC) */
 
 #ifdef CONFIG_CRYPTO_DRBG_HMAC
+#define CRYPTO_DRBG_HMAC_STRING "HMAC "
 /* update function of HMAC DRBG as defined in 10.1.2.2 */
 static int drbg_hmac_update(struct drbg_state *drbg, struct list_head *seed,
 			    int reseed)
@@ -834,6 +841,7 @@ static struct drbg_state_ops drbg_hmac_ops = {
  ******************************************************************/
 
 #ifdef CONFIG_CRYPTO_DRBG_HASH
+#define CRYPTO_DRBG_HASH_STRING "HASH "
 /*
  * scratchpad usage: as drbg_hash_update and drbg_hash_df are used
  * interlinked, the scratchpad is used as follows:
@@ -1865,7 +1873,7 @@ static inline int __init drbg_healthcheck_sanity(void)
 
 #ifdef CONFIG_CRYPTO_DRBG_CTR
 	drbg_convert_tfm_core("drbg_nopr_ctr_aes128", &coreref, &pr);
-#elif CONFIG_CRYPTO_DRBG_HASH
+#elif defined CONFIG_CRYPTO_DRBG_HASH
 	drbg_convert_tfm_core("drbg_nopr_sha256", &coreref, &pr);
 #else
 	drbg_convert_tfm_core("drbg_nopr_hmac_sha256", &coreref, &pr);
@@ -2005,18 +2013,29 @@ void __exit drbg_exit(void)
 	crypto_unregister_algs(drbg_algs, (ARRAY_SIZE(drbg_cores) * 2));
 }
 
-module_init(drbg_init);
 module_exit(drbg_exit);
+#endif /* !defined(CONFIG_CRYPTO_DRBG_HASH) && \
+	  !defined(CONFIG_CRYPTO_DRBG_HMAC) && \
+	  !defined(CONFIG_CRYPTO_DRBG_CTR) */
+
+module_init(drbg_init);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
-MODULE_DESCRIPTION("NIST SP800-90A Deterministic Random Bit Generator (DRBG) using following cores:"
-#ifdef CONFIG_CRYPTO_DRBG_HMAC
-"HMAC "
+#ifndef CRYPTO_DRBG_NONE_STRING
+#define CRYPTO_DRBG_NONE_STRING ""
 #endif
-#ifdef CONFIG_CRYPTO_DRBG_HASH
-"Hash "
+#ifndef CRYPTO_DRBG_HASH_STRING
+#define CRYPTO_DRBG_HASH_STRING ""
 #endif
-#ifdef CONFIG_CRYPTO_DRBG_CTR
-"CTR"
+#ifndef CRYPTO_DRBG_HMAC_STRING
+#define CRYPTO_DRBG_HMAC_STRING ""
+#endif
+#ifndef CRYPTO_DRBG_CTR_STRING
+#define CRYPTO_DRBG_CTR_STRING ""
 #endif
-);
+MODULE_DESCRIPTION("NIST SP800-90A Deterministic Random Bit Generator (DRBG) "
+		   "using following cores: "
+		   CRYPTO_DRBG_NONE_STRING
+		   CRYPTO_DRBG_HMAC_STRING
+		   CRYPTO_DRBG_HASH_STRING
+		   CRYPTO_DRBG_CTR_STRING);
-- 
1.9.3

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi Stephan,

On Sat, 28 Jun 2014 22:00:07 +0200 Stephan Mueller <smueller@chronox.de> wrote:
>
> diff --git a/crypto/drbg.c b/crypto/drbg.c
> index 6679a26..03a230e 100644
> --- a/crypto/drbg.c
> +++ b/crypto/drbg.c
> @@ -102,8 +102,13 @@
>  #if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
>  	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
>  	!defined(CONFIG_CRYPTO_DRBG_CTR)
> -#warning "The DRBG code is useless without compiling at least one DRBG type"
> -#endif
> +#define CRYPTO_DRBG_NONE_STRING "none "
> +static int __init drbg_init(void)
> +{
> +	pr_warn("DRBG: no DRBG core was compiled!\n");
> +	return -EFAULT;
> +}
> +#else

Wouldn't this be better handled by Kconfig so that we don't even try to
build this unless one of the required core modules is chosen?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Stephan Mueller]

Am Sonntag, 29. Juni 2014, 12:20:15 schrieb Stephen Rothwell:

Hi Stephen,

> Hi Stephan,
> 
> On Sat, 28 Jun 2014 22:00:07 +0200 Stephan Mueller <smueller@chronox.de> 
wrote:
> > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > index 6679a26..03a230e 100644
> > --- a/crypto/drbg.c
> > +++ b/crypto/drbg.c
> > @@ -102,8 +102,13 @@
> > 
> >  #if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
> >  
> >  	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
> >  	!defined(CONFIG_CRYPTO_DRBG_CTR)
> > 
> > -#warning "The DRBG code is useless without compiling at least one DRBG
> > type" -#endif
> > +#define CRYPTO_DRBG_NONE_STRING "none "
> > +static int __init drbg_init(void)
> > +{
> > +	pr_warn("DRBG: no DRBG core was compiled!\n");
> > +	return -EFAULT;
> > +}
> > +#else
> 
> Wouldn't this be better handled by Kconfig so that we don't even try to
> build this unless one of the required core modules is chosen?

I tried that, but it seems that my Kconfig Foo is not too well: adding the 
DRBG cores to the depends line of CRYPTO_DRBG as indicated in the following, I 
have a circular dependency. With that circular dependency, the DRBG entries do 
not show up in make menuconfig.

menuconfig CRYTPO_DRBG
        tristate "NIST SP800-90A DRBG"
        depends on CRYPTO && (CRYPTO_DRBG_HMAC || CRYPTO_DRBG_CTR || 
CRYPTO_DRBG_HASH)
...

if CRYTPO_DRBG

config CRYPTO_DRBG_HMAC
        bool "Enable HMAC DRBG"
        default y
        depends on CRYTPO_DRBG

Do you have a working solution in mind? The goal is that once CRYPTO_DRBG is 
selected, at least one of the DRBG cores must be selected.

Thanks
Stephan
-- 
| Cui bono? |

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Randy Dunlap]

On 06/28/14 22:07, Stephan Mueller wrote:
> Am Sonntag, 29. Juni 2014, 12:20:15 schrieb Stephen Rothwell:
> 
> Hi Stephen,
> 
>> Hi Stephan,
>>
>> On Sat, 28 Jun 2014 22:00:07 +0200 Stephan Mueller <smueller@chronox.de> 
> wrote:
>>> diff --git a/crypto/drbg.c b/crypto/drbg.c
>>> index 6679a26..03a230e 100644
>>> --- a/crypto/drbg.c
>>> +++ b/crypto/drbg.c
>>> @@ -102,8 +102,13 @@
>>>
>>>  #if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
>>>  
>>>  	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
>>>  	!defined(CONFIG_CRYPTO_DRBG_CTR)
>>>
>>> -#warning "The DRBG code is useless without compiling at least one DRBG
>>> type" -#endif
>>> +#define CRYPTO_DRBG_NONE_STRING "none "
>>> +static int __init drbg_init(void)
>>> +{
>>> +	pr_warn("DRBG: no DRBG core was compiled!\n");
>>> +	return -EFAULT;
>>> +}
>>> +#else
>>
>> Wouldn't this be better handled by Kconfig so that we don't even try to
>> build this unless one of the required core modules is chosen?
> 
> I tried that, but it seems that my Kconfig Foo is not too well: adding the 
> DRBG cores to the depends line of CRYPTO_DRBG as indicated in the following, I 
> have a circular dependency. With that circular dependency, the DRBG entries do 
> not show up in make menuconfig.
> 
> menuconfig CRYTPO_DRBG
>         tristate "NIST SP800-90A DRBG"
>         depends on CRYPTO && (CRYPTO_DRBG_HMAC || CRYPTO_DRBG_CTR || 
> CRYPTO_DRBG_HASH)
> ...
> 
> if CRYTPO_DRBG
> 
> config CRYPTO_DRBG_HMAC
>         bool "Enable HMAC DRBG"
>         default y
>         depends on CRYTPO_DRBG
> 
> Do you have a working solution in mind? The goal is that once CRYPTO_DRBG is 
> selected, at least one of the DRBG cores must be selected.

That sounds like a 'choice' Kconfig could be used.
Have you looked at that possibility?

See Documentation/kbuild/kconfig-language.txt and search for 'choice'.


-- 
~Randy

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Stephan Mueller]

Am Sonntag, 29. Juni 2014, 00:41:22 schrieb Randy Dunlap:

Hi Randy,

> On 06/28/14 22:07, Stephan Mueller wrote:
> > Am Sonntag, 29. Juni 2014, 12:20:15 schrieb Stephen Rothwell:
> > 
> > Hi Stephen,
> > 
> >> Hi Stephan,
> >> 
> >> On Sat, 28 Jun 2014 22:00:07 +0200 Stephan Mueller <smueller@chronox.de>
> > 
> > wrote:
> >>> diff --git a/crypto/drbg.c b/crypto/drbg.c
> >>> index 6679a26..03a230e 100644
> >>> --- a/crypto/drbg.c
> >>> +++ b/crypto/drbg.c
> >>> @@ -102,8 +102,13 @@
> >>> 
> >>>  #if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
> >>>  
> >>>  	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
> >>>  	!defined(CONFIG_CRYPTO_DRBG_CTR)
> >>> 
> >>> -#warning "The DRBG code is useless without compiling at least one DRBG
> >>> type" -#endif
> >>> +#define CRYPTO_DRBG_NONE_STRING "none "
> >>> +static int __init drbg_init(void)
> >>> +{
> >>> +	pr_warn("DRBG: no DRBG core was compiled!\n");
> >>> +	return -EFAULT;
> >>> +}
> >>> +#else
> >> 
> >> Wouldn't this be better handled by Kconfig so that we don't even try to
> >> build this unless one of the required core modules is chosen?
> > 
> > I tried that, but it seems that my Kconfig Foo is not too well: adding the
> > DRBG cores to the depends line of CRYPTO_DRBG as indicated in the
> > following, I have a circular dependency. With that circular dependency,
> > the DRBG entries do not show up in make menuconfig.
> > 
> > menuconfig CRYTPO_DRBG
> > 
> >         tristate "NIST SP800-90A DRBG"
> >         depends on CRYPTO && (CRYPTO_DRBG_HMAC || CRYPTO_DRBG_CTR ||
> > 
> > CRYPTO_DRBG_HASH)
> > ...
> > 
> > if CRYTPO_DRBG
> > 
> > config CRYPTO_DRBG_HMAC
> > 
> >         bool "Enable HMAC DRBG"
> >         default y
> >         depends on CRYTPO_DRBG
> > 
> > Do you have a working solution in mind? The goal is that once CRYPTO_DRBG
> > is selected, at least one of the DRBG cores must be selected.
> 
> That sounds like a 'choice' Kconfig could be used.
> Have you looked at that possibility?
> 
> See Documentation/kbuild/kconfig-language.txt and search for 'choice'.

When looking into the documentation and trying it I found:

- bool choices allow me to only select one option, and only one

- tristate choices allow me to only select one option, if the initial tristate 
is set to yes. If the initial tristate is set to module, it allows zero to all 
options to be set.

That said, neither covers my requirement here: require that at least one 
option is set, but allow more options.

Thanks
Stephan
-- 
| Cui bono? |

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Herbert Xu]

On Sun, Jun 29, 2014 at 01:37:05PM +0200, Stephan Mueller wrote:
>
> When looking into the documentation and trying it I found:
> 
> - bool choices allow me to only select one option, and only one
> 
> - tristate choices allow me to only select one option, if the initial tristate 
> is set to yes. If the initial tristate is set to module, it allows zero to all 
> options to be set.
> 
> That said, neither covers my requirement here: require that at least one 
> option is set, but allow more options.

I have added the following patch to solve this problem.

Please respin your patch against this.

commit f2c89a10de4fd123a3d15223d26994f2fe1b95d8
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Fri Jul 4 22:15:08 2014 +0800

    crypto: drbg - Use Kconfig to ensure at least one RNG option is set
    
    This patch removes the build-time test that ensures at least one RNG
    is set.  Instead we will simply not build drbg if no options are set
    through Kconfig.
    
    This also fixes a typo in the name of the Kconfig option CRYTPO_DRBG
    (should be CRYPTO_DRBG).
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1dca374..6345c47 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -23,7 +23,7 @@ comment "Crypto core or helper"
 
 config CRYPTO_FIPS
 	bool "FIPS 200 compliance"
-	depends on (CRYPTO_ANSI_CPRNG || CRYTPO_DRBG) && !CRYPTO_MANAGER_DISABLE_TESTS
+	depends on (CRYPTO_ANSI_CPRNG || CRYPTO_DRBG) && !CRYPTO_MANAGER_DISABLE_TESTS
 	depends on MODULE_SIG
 	help
 	  This options enables the fips boot option which is
@@ -1394,39 +1394,39 @@ config CRYPTO_ANSI_CPRNG
 	  ANSI X9.31 A.2.4. Note that this option must be enabled if
 	  CRYPTO_FIPS is selected
 
-menuconfig CRYTPO_DRBG
+menuconfig CRYPTO_DRBG_MENU
 	tristate "NIST SP800-90A DRBG"
-	depends on CRYPTO
-	select CRYPTO_RNG
 	help
 	  NIST SP800-90A compliant DRBG. In the following submenu, one or
 	  more of the DRBG types must be selected.
 
-if CRYTPO_DRBG
+if CRYPTO_DRBG_MENU
 
 config CRYPTO_DRBG_HMAC
 	bool "Enable HMAC DRBG"
 	default y
-	depends on CRYTPO_DRBG
 	select CRYPTO_HMAC
 	help
 	  Enable the HMAC DRBG variant as defined in NIST SP800-90A.
 
 config CRYPTO_DRBG_HASH
 	bool "Enable Hash DRBG"
-	depends on CRYTPO_DRBG
 	select CRYPTO_HASH
 	help
 	  Enable the Hash DRBG variant as defined in NIST SP800-90A.
 
 config CRYPTO_DRBG_CTR
 	bool "Enable CTR DRBG"
-	depends on CRYTPO_DRBG
 	select CRYPTO_AES
 	help
 	  Enable the CTR DRBG variant as defined in NIST SP800-90A.
 
-endif #CRYTPO_DRBG
+config CRYPTO_DRBG
+	tristate
+	default CRYPTO_DRBG_MENU if (CRYPTO_DRBG_HMAC || CRYPTO_DRBG_HASH || CRYPTO_DRBG_CTR)
+	select CRYPTO_RNG
+
+endif	# if CRYPTO_DRBG_MENU
 
 config CRYPTO_USER_API
 	tristate
diff --git a/crypto/Makefile b/crypto/Makefile
index bfa94fa..cfa57b3 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -92,7 +92,7 @@ obj-$(CONFIG_CRYPTO_842) += 842.o
 obj-$(CONFIG_CRYPTO_RNG2) += rng.o
 obj-$(CONFIG_CRYPTO_RNG2) += krng.o
 obj-$(CONFIG_CRYPTO_ANSI_CPRNG) += ansi_cprng.o
-obj-$(CONFIG_CRYTPO_DRBG) += drbg.o
+obj-$(CONFIG_CRYPTO_DRBG) += drbg.o
 obj-$(CONFIG_CRYPTO_TEST) += tcrypt.o
 obj-$(CONFIG_CRYPTO_GHASH) += ghash-generic.o
 obj-$(CONFIG_CRYPTO_USER_API) += af_alg.o
diff --git a/crypto/drbg.c b/crypto/drbg.c
index d6621a6..acc7523 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -99,12 +99,6 @@
 
 #include <crypto/drbg.h>
 
-#if !defined(CONFIG_CRYPTO_DRBG_HASH) && \
-	!defined(CONFIG_CRYPTO_DRBG_HMAC) && \
-	!defined(CONFIG_CRYPTO_DRBG_CTR)
-#warning "The DRBG code is useless without compiling at least one DRBG type"
-#endif
-
 /***************************************************************
  * Backend cipher definitions available to DRBG
  ***************************************************************/

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros

[Stephan Mueller]

Am Freitag, 4. Juli 2014, 22:15:41 schrieb Herbert Xu:

Hi Herbert,

> On Sun, Jun 29, 2014 at 01:37:05PM +0200, Stephan Mueller wrote:
> > When looking into the documentation and trying it I found:
> > 
> > - bool choices allow me to only select one option, and only one
> > 
> > - tristate choices allow me to only select one option, if the initial
> > tristate is set to yes. If the initial tristate is set to module, it
> > allows zero to all options to be set.
> > 
> > That said, neither covers my requirement here: require that at least one
> > option is set, but allow more options.
> 
> I have added the following patch to solve this problem.

Thank you very much for the help. I will test it once it is present on the 
server.
> 
> Please respin your patch against this.

All patches that have been sent so far and not applied will be respun around 
the new code level once the updated code appears on the server.

-- 
Ciao
Stephan

[PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

The initial format strings caused warnings on several architectures. The
updated format strings now match the variable types.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 03a230e..4593b3c 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1109,7 +1109,7 @@ static int drbg_seed(struct drbg_state *drbg, struct drbg_string *pers,
 
 	/* 9.1 / 9.2 / 9.3.1 step 3 */
 	if (pers && pers->len > (drbg_max_addtl(drbg))) {
-		pr_devel("DRBG: personalization string too long %lu\n",
+		pr_devel("DRBG: personalization string too long %zu\n",
 			 pers->len);
 		return -EINVAL;
 	}
@@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
 
 	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
 		pr_info("DRBG: Cannot register all DRBG types"
-			"(slots needed: %lu, slots available: %lu)\n",
-			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
+			"(slots needed: %u, slots available: %u)\n",
+			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
+			(unsigned int)ARRAY_SIZE(drbg_algs));
 		return ret;
 	}
 
-- 
1.9.3

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephen Rothwell]

Hi Stephan,

On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de> wrote:
>
> @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
>  
>  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
>  		pr_info("DRBG: Cannot register all DRBG types"
> -			"(slots needed: %lu, slots available: %lu)\n",
> -			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
> +			"(slots needed: %u, slots available: %u)\n",
> +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> +			(unsigned int)ARRAY_SIZE(drbg_algs));

Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
need no casts, but need to us %zu in the format string.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:

Hi Stephen,

> Hi Stephan,
> 
> On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de> 
wrote:
> > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > 
> >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> >  	
> >  		pr_info("DRBG: Cannot register all DRBG types"
> > 
> > -			"(slots needed: %lu, slots available: %lu)\n",
> > -			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
> > +			"(slots needed: %u, slots available: %u)\n",
> > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> 
> Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
> need no casts, but need to us %zu in the format string.

Unfortunately not at all. On my x86_64, I get the compiler warning that 
ARRAY_SIZE is a long unsigned int without the cast.

Ciao
Stephan
-- 
| Cui bono? |

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Joe Perches]

On Sun, 2014-06-29 at 05:46 +0200, Stephan Mueller wrote:
> Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:
> 
> Hi Stephen,
> 
> > Hi Stephan,
> > 
> > On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de> 
> wrote:
> > > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > > 
> > >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> > >  	
> > >  		pr_info("DRBG: Cannot register all DRBG types"
> > > 
> > > -			"(slots needed: %lu, slots available: %lu)\n",
> > > -			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
> > > +			"(slots needed: %u, slots available: %u)\n",
> > > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> > 
> > Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
> > need no casts, but need to us %zu in the format string.
> 
> Unfortunately not at all. On my x86_64, I get the compiler warning that 
> ARRAY_SIZE is a long unsigned int without the cast.

This should fix that.
---
 include/linux/kernel.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 6e3d497..58bc57d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -51,7 +51,8 @@
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
 
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+#define ARRAY_SIZE(arr)						\
+	(sizeof(arr) / sizeof((arr)[0]) + (size_t)__must_be_array(arr))
 
 /*
  * This looks more complex than it should be. But we need to

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

Am Samstag, 28. Juni 2014, 20:53:19 schrieb Joe Perches:

Hi Joe,

> On Sun, 2014-06-29 at 05:46 +0200, Stephan Mueller wrote:
> > Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:
> > 
> > Hi Stephen,
> > 
> > > Hi Stephan,
> > > 
> > > On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de>
> > 
> > wrote:
> > > > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > > > 
> > > >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> > > >  	
> > > >  		pr_info("DRBG: Cannot register all DRBG types"
> > > > 
> > > > -			"(slots needed: %lu, slots available: %lu)\n",
> > > > -			ARRAY_SIZE(drbg_cores) * 2, 
ARRAY_SIZE(drbg_algs));
> > > > +			"(slots needed: %u, slots available: %u)\n",
> > > > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > > > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> > > 
> > > Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
> > > need no casts, but need to us %zu in the format string.
> > 
> > Unfortunately not at all. On my x86_64, I get the compiler warning that
> > ARRAY_SIZE is a long unsigned int without the cast.
> 
> This should fix that.
> ---
>  include/linux/kernel.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 6e3d497..58bc57d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -51,7 +51,8 @@
>  #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), 
(a)))
>  #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
> 
> -#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr)) +#define ARRAY_SIZE(arr)				
		\
> +	(sizeof(arr) / sizeof((arr)[0]) + (size_t)__must_be_array(arr))
> 
>  /*
>   * This looks more complex than it should be. But we need to


Sure, that fixes it such that I need to use %zu in the format string.

But wouldn't that change have riple effects to all use cases of ARRAY_SIZE at 
least on 32 bit systems (i.e. current implementation returns a 32 bit integer, 
but the new version returns a 64 bit integer)? If so, I am wondering whether 
this change can be made with this oneliner.

Ciao
Stephan
-- 
| Cui bono? |

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Dan Carpenter]

On Sat, Jun 28, 2014 at 08:53:19PM -0700, Joe Perches wrote:
> On Sun, 2014-06-29 at 05:46 +0200, Stephan Mueller wrote:
> > Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:
> > 
> > Hi Stephen,
> > 
> > > Hi Stephan,
> > > 
> > > On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de> 
> > wrote:
> > > > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > > > 
> > > >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> > > >  	
> > > >  		pr_info("DRBG: Cannot register all DRBG types"
> > > > 
> > > > -			"(slots needed: %lu, slots available: %lu)\n",
> > > > -			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
> > > > +			"(slots needed: %u, slots available: %u)\n",
> > > > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > > > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> > > 
> > > Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
> > > need no casts, but need to us %zu in the format string.
> > 
> > Unfortunately not at all. On my x86_64, I get the compiler warning that 
> > ARRAY_SIZE is a long unsigned int without the cast.
> 
> This should fix that.
> ---
>  include/linux/kernel.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 6e3d497..58bc57d 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -51,7 +51,8 @@
>  #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
>  #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
>  
> -#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
> +#define ARRAY_SIZE(arr)						\
> +	(sizeof(arr) / sizeof((arr)[0]) + (size_t)__must_be_array(arr))


This change is a no-op isn't it?  I think Stephen Rothwell's suggestion
is correct.  In linux-next this was changed to %lu which also works...

Are there arches %zu and %lu are different?

regards,
dan carpenter

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Joe Perches]

On Fri, 2014-07-04 at 14:21 +0300, Dan Carpenter wrote:
> On Sat, Jun 28, 2014 at 08:53:19PM -0700, Joe Perches wrote:
> > On Sun, 2014-06-29 at 05:46 +0200, Stephan Mueller wrote:
> > > Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:
> > > 
> > > Hi Stephen,
> > > 
> > > > Hi Stephan,
> > > > 
> > > > On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller <smueller@chronox.de> 
> > > wrote:
> > > > > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > > > > 
> > > > >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> > > > >  	
> > > > >  		pr_info("DRBG: Cannot register all DRBG types"
> > > > > 
> > > > > -			"(slots needed: %lu, slots available: %lu)\n",
> > > > > -			ARRAY_SIZE(drbg_cores) * 2, ARRAY_SIZE(drbg_algs));
> > > > > +			"(slots needed: %u, slots available: %u)\n",
> > > > > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > > > > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> > > > 
> > > > Doesn't ARRAY_SIZE() always return a size_t?  In which case surely we
> > > > need no casts, but need to us %zu in the format string.
> > > 
> > > Unfortunately not at all. On my x86_64, I get the compiler warning that 
> > > ARRAY_SIZE is a long unsigned int without the cast.

It doesn't seem to for 4.8.
Is there some specific gcc version where this occurs?

> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
[]
> > -#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
> > +#define ARRAY_SIZE(arr)						\
> > +	(sizeof(arr) / sizeof((arr)[0]) + (size_t)__must_be_array(arr))
> 
> 
> This change is a no-op isn't it?

Yes, it is.  Dumb idea.
Assuming there's some odd promotion, size_t should have been around the
whole thing
#define ARRAY_SIZE(arr)						\
	((size_t)((sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)))

> I think Stephen Rothwell's suggestion
> is correct.  In linux-next this was changed to %lu which also works...
> 
> Are there arches %zu and %lu are different?

I get the same output types and error warnings compiling this
either -m32 or -m64

#include <stdio.h>
#include <stdlib.h>

#define typecheck(type, x)			\
({						\
	type __dummy;				\
	typeof(x) __dummy2;			\
	(void)(&__dummy == &__dummy2);		\
	1;					\
})

#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))

#define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))

int main(int argc, char **argv)
{
	char foo[100];
	size_t array = sizeof(foo);
	typeof (ARRAY_SIZE(foo)) member = ARRAY_SIZE(foo);

	int member1 = ARRAY_SIZE(foo);
	size_t member2 = ARRAY_SIZE(foo);

	typecheck(size_t, member);
	typecheck(int, member);

	typecheck(size_t, member1);
	typecheck(int, member1);

	typecheck(size_t, member2);
	typecheck(int, member2);

	printf("array: %zu, member: %zu\n", array, (int)member);

	return 0;
}

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

Am Freitag, 4. Juli 2014, 09:57:20 schrieb Joe Perches:

Hi Joe,

> On Fri, 2014-07-04 at 14:21 +0300, Dan Carpenter wrote:
> > On Sat, Jun 28, 2014 at 08:53:19PM -0700, Joe Perches wrote:
> > > On Sun, 2014-06-29 at 05:46 +0200, Stephan Mueller wrote:
> > > > Am Sonntag, 29. Juni 2014, 12:24:02 schrieb Stephen Rothwell:
> > > > 
> > > > Hi Stephen,
> > > > 
> > > > > Hi Stephan,
> > > > > 
> > > > > On Sat, 28 Jun 2014 22:01:46 +0200 Stephan Mueller
> > > > > <smueller@chronox.de>
> > > > 
> > > > wrote:
> > > > > > @@ -1987,8 +1987,9 @@ static int __init drbg_init(void)
> > > > > > 
> > > > > >  	if (ARRAY_SIZE(drbg_cores) * 2 > ARRAY_SIZE(drbg_algs)) {
> > > > > >  	
> > > > > >  		pr_info("DRBG: Cannot register all DRBG types"
> > > > > > 
> > > > > > -			"(slots needed: %lu, slots available: %lu)\n",
> > > > > > -			ARRAY_SIZE(drbg_cores) * 2, 
ARRAY_SIZE(drbg_algs));
> > > > > > +			"(slots needed: %u, slots available: %u)\n",
> > > > > > +			(unsigned int)ARRAY_SIZE(drbg_cores) * 2,
> > > > > > +			(unsigned int)ARRAY_SIZE(drbg_algs));
> > > > > 
> > > > > Doesn't ARRAY_SIZE() always return a size_t?  In which case surely
> > > > > we
> > > > > need no casts, but need to us %zu in the format string.
> > > > 
> > > > Unfortunately not at all. On my x86_64, I get the compiler warning
> > > > that
> > > > ARRAY_SIZE is a long unsigned int without the cast.
> 
> It doesn't seem to for 4.8.
> Is there some specific gcc version where this occurs?

$ LANG=en_US gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.8.3/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --
infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --
enable-bootstrap --enable-shared --enable-threads=posix --enable-
checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-
exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-
hash-style=gnu --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto 
--enable-plugin --enable-initfini-array --enable-java-awt=gtk --disable-dssi 
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-
multifile --enable-java-maintainer-mode --with-ecj-
jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-
isl=/builddir/build/BUILD/gcc-4.8.3-20140624/obj-x86_64-redhat-linux/isl-
install --with-cloog=/builddir/build/BUILD/gcc-4.8.3-20140624/obj-x86_64-
redhat-linux/cloog-install --with-tune=generic --with-arch_32=i686 --
build=x86_64-redhat-linux
Thread model: posix
gcc version 4.8.3 20140624 (Red Hat 4.8.3-1) (GCC)

Note, I get a warning when compiling 64 bit with %u and without the unsigned 
int cast. Initially I had %lu without the cast which worked on my x86_64, but 
it was reported that this usage caused a warning on 32 bit. So, there is a 
different type of ARRAY_SIZE.

> I get the same output types and error warnings compiling this
> either -m32 or -m64

I do not get a warning for the ARRAY_SIZE thing with your code with either -
m32 or -m64 (I get other warnings though, which should not be of interest 
here).

And I also get the same output. Yet I am not sure how that code can be 
compared to the code in the kernel.

-- 
Ciao
Stephan

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Joe Perches]

On Sat, 2014-07-05 at 01:57 +0200, Stephan Mueller wrote:
> And I also get the same output. Yet I am not sure how that code can be 
> compared to the code in the kernel.

What that code shows is that the ARRAY_SIZE
type is size_t.

The difference is ARRAY_SIZE in the kernel
should be output with %zu.

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

Am Freitag, 4. Juli 2014, 17:09:33 schrieb Joe Perches:

Hi Joe,

> On Sat, 2014-07-05 at 01:57 +0200, Stephan Mueller wrote:
> > And I also get the same output. Yet I am not sure how that code can be
> > compared to the code in the kernel.
> 
> What that code shows is that the ARRAY_SIZE
> type is size_t.
> 
> The difference is ARRAY_SIZE in the kernel
> should be output with %zu.

Using %zu works without a warning on my 64 bit machine. So you are saying that 
ARRAY_SIZE will always be size_t on every architecture? If yes, I will update 
my patch.


-- 
Ciao
Stephan

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Joe Perches]

On Sat, 2014-07-05 at 02:15 +0200, Stephan Mueller wrote:
> Am Freitag, 4. Juli 2014, 17:09:33 schrieb Joe Perches:
> > On Sat, 2014-07-05 at 01:57 +0200, Stephan Mueller wrote:
> > > And I also get the same output. Yet I am not sure how that code can be
> > > compared to the code in the kernel.
> > 
> > What that code shows is that the ARRAY_SIZE
> > type is size_t.
> > 
> > The difference is ARRAY_SIZE in the kernel
> > should be output with %zu.
> 
> Using %zu works without a warning on my 64 bit machine. So you are saying that 
> ARRAY_SIZE will always be size_t on every architecture? If yes, I will update 
> my patch.

Hi again.

ARRAY_SIZE is size_t in all architectures.

Re: [PATCH 3/4] DRBG: Fix format string for debugging statements

[Stephan Mueller]

Am Freitag, 4. Juli 2014, 17:24:09 schrieb Joe Perches:

Hi Joe,

> On Sat, 2014-07-05 at 02:15 +0200, Stephan Mueller wrote:
> > Am Freitag, 4. Juli 2014, 17:09:33 schrieb Joe Perches:
> > > On Sat, 2014-07-05 at 01:57 +0200, Stephan Mueller wrote:
> > > > And I also get the same output. Yet I am not sure how that code can be
> > > > compared to the code in the kernel.
> > > 
> > > What that code shows is that the ARRAY_SIZE
> > > type is size_t.
> > > 
> > > The difference is ARRAY_SIZE in the kernel
> > > should be output with %zu.
> > 
> > Using %zu works without a warning on my 64 bit machine. So you are saying
> > that ARRAY_SIZE will always be size_t on every architecture? If yes, I
> > will update my patch.
> 
> Hi again.
> 
> ARRAY_SIZE is size_t in all architectures.

Thank you very much, this is the answer I was looking for :-)

I will update the patch accordingly.

-- 
Ciao
Stephan

[PATCH 4/4] DRBG: Call CTR DRBG DF function only once

[Stephan Mueller]

The CTR DRBG requires the update function to be called twice when
generating a random number. In both cases, update function must process
the additional information string by using the DF function. As the DF
produces the same result in both cases, we can save one invocation of
the DF function when the first DF function result is reused.

The result of the DF function is stored in the scratchpad storage. The
patch ensures that the scratchpad is not cleared when we want to reuse
the DF result. For achieving this, the CTR DRBG update function must
know by whom and in which scenario it is called. This information is
provided with the reseed parameter to the update function.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 4593b3c..53ff20d 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -565,7 +565,21 @@ out:
 	return ret;
 }
 
-/* update function of CTR DRBG as defined in 10.2.1.2 */
+/*
+ * update function of CTR DRBG as defined in 10.2.1.2
+ *
+ * The reseed variable has an enhanced meaning compared to the update
+ * functions of the other DRBGs as follows:
+ * 0 => initial seed from initialization
+ * 1 => reseed via drbg_seed
+ * 2 => first invocation from drbg_ctr_update when addtl is present. In
+ *      this case, the df_data scratchpad is not deleted so that it is
+ *      available for another calls to prevent calling the DF function
+ *      again.
+ * 3 => second invocation from drbg_ctr_update. When the update function
+ *      was called with addtl, the df_data memory already contains the
+ *      DFed addtl information and we do not need to call DF again.
+ */
 static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 			   int reseed)
 {
@@ -580,7 +594,8 @@ static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 	unsigned char prefix = DRBG_PREFIX1;
 
 	memset(temp, 0, drbg_statelen(drbg) + drbg_blocklen(drbg));
-	memset(df_data, 0, drbg_statelen(drbg));
+	if (3 > reseed)
+		memset(df_data, 0, drbg_statelen(drbg));
 
 	/* 10.2.1.3.2 step 2 and 10.2.1.4.2 step 2 */
 	if (seed) {
@@ -622,7 +637,8 @@ static int drbg_ctr_update(struct drbg_state *drbg, struct list_head *seed,
 
 out:
 	memset(temp, 0, drbg_statelen(drbg) + drbg_blocklen(drbg));
-	memset(df_data, 0, drbg_statelen(drbg));
+	if (2 != reseed)
+		memset(df_data, 0, drbg_statelen(drbg));
 	return ret;
 }
 
@@ -647,7 +663,7 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 		LIST_HEAD(addtllist);
 
 		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_ctr_update(drbg, &addtllist, 1);
+		ret = drbg_ctr_update(drbg, &addtllist, 2);
 		if (ret)
 			return 0;
 	}
@@ -678,21 +694,8 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
 			drbg_add_buf(drbg->V, drbg_blocklen(drbg), &prefix, 1);
 	}
 
-	/*
-	 * 10.2.1.5.2 step 6
-	 * The following call invokes the DF function again which could be
-	 * optimized. In step 2, the "additional_input" after step 2 is the
-	 * output of the DF function. If this result would be saved, the DF
-	 * function would not need to be invoked again at this point.
-	 */
-	if (addtl && 0 < addtl->len) {
-		LIST_HEAD(addtllist);
-
-		list_add_tail(&addtl->list, &addtllist);
-		ret = drbg_ctr_update(drbg, &addtllist, 1);
-	} else {
-		ret = drbg_ctr_update(drbg, NULL, 1);
-	}
+	/* 10.2.1.5.2 step 6 */
+	ret = drbg_ctr_update(drbg, NULL, 3);
 	if (ret)
 		len = ret;
 
-- 
1.9.3