* [PATCH 0/4] DRBG: Fixes for sparse tool reports
@ 2014-06-28 19:57 Stephan Mueller
2014-06-28 19:58 ` [PATCH 1/4] DRBG: use of kernel linked list Stephan Mueller
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: Stephan Mueller @ 2014-06-28 19:57 UTC (permalink / raw)
To: Herbert Xu
Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto,
Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/4] DRBG: use of kernel linked list 2014-06-28 19:57 [PATCH 0/4] DRBG: Fixes for sparse tool reports Stephan Mueller @ 2014-06-28 19:58 ` Stephan Mueller 2014-07-04 14:11 ` Herbert Xu 2014-06-28 20:00 ` [PATCH 2/4] DRBG: cleanup of preprocessor macros Stephan Mueller ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-28 19:58 UTC (permalink / raw) To: Herbert Xu Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] DRBG: use of kernel linked list 2014-06-28 19:58 ` [PATCH 1/4] DRBG: use of kernel linked list Stephan Mueller @ 2014-07-04 14:11 ` Herbert Xu 0 siblings, 0 replies; 24+ messages in thread From: Herbert Xu @ 2014-07-04 14:11 UTC (permalink / raw) To: Stephan Mueller Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-28 19:57 [PATCH 0/4] DRBG: Fixes for sparse tool reports Stephan Mueller 2014-06-28 19:58 ` [PATCH 1/4] DRBG: use of kernel linked list Stephan Mueller @ 2014-06-28 20:00 ` Stephan Mueller 2014-06-29 2:20 ` Stephen Rothwell 2014-06-28 20:01 ` [PATCH 3/4] DRBG: Fix format string for debugging statements Stephan Mueller 2014-06-28 20:04 ` [PATCH 4/4] DRBG: Call CTR DRBG DF function only once Stephan Mueller 3 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-28 20:00 UTC (permalink / raw) To: Herbert Xu Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-28 20:00 ` [PATCH 2/4] DRBG: cleanup of preprocessor macros Stephan Mueller @ 2014-06-29 2:20 ` Stephen Rothwell 2014-06-29 5:07 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Stephen Rothwell @ 2014-06-29 2:20 UTC (permalink / raw) To: Stephan Mueller Cc: Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-29 2:20 ` Stephen Rothwell @ 2014-06-29 5:07 ` Stephan Mueller 2014-06-29 7:41 ` Randy Dunlap 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-29 5:07 UTC (permalink / raw) To: Stephen Rothwell Cc: Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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? | ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-29 5:07 ` Stephan Mueller @ 2014-06-29 7:41 ` Randy Dunlap 2014-06-29 11:37 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Randy Dunlap @ 2014-06-29 7:41 UTC (permalink / raw) To: Stephan Mueller, Stephen Rothwell Cc: Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-29 7:41 ` Randy Dunlap @ 2014-06-29 11:37 ` Stephan Mueller 2014-07-04 14:15 ` Herbert Xu 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-29 11:37 UTC (permalink / raw) To: Randy Dunlap Cc: Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, linux-next, linux-kernel 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? | ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-06-29 11:37 ` Stephan Mueller @ 2014-07-04 14:15 ` Herbert Xu 2014-07-05 0:03 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Herbert Xu @ 2014-07-04 14:15 UTC (permalink / raw) To: Stephan Mueller Cc: Randy Dunlap, Stephen Rothwell, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] DRBG: cleanup of preprocessor macros 2014-07-04 14:15 ` Herbert Xu @ 2014-07-05 0:03 ` Stephan Mueller 0 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-07-05 0:03 UTC (permalink / raw) To: Herbert Xu Cc: Randy Dunlap, Stephen Rothwell, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-28 19:57 [PATCH 0/4] DRBG: Fixes for sparse tool reports Stephan Mueller 2014-06-28 19:58 ` [PATCH 1/4] DRBG: use of kernel linked list Stephan Mueller 2014-06-28 20:00 ` [PATCH 2/4] DRBG: cleanup of preprocessor macros Stephan Mueller @ 2014-06-28 20:01 ` Stephan Mueller 2014-06-29 2:24 ` Stephen Rothwell 2014-06-28 20:04 ` [PATCH 4/4] DRBG: Call CTR DRBG DF function only once Stephan Mueller 3 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-28 20:01 UTC (permalink / raw) To: Herbert Xu Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-28 20:01 ` [PATCH 3/4] DRBG: Fix format string for debugging statements Stephan Mueller @ 2014-06-29 2:24 ` Stephen Rothwell 2014-06-29 3:46 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Stephen Rothwell @ 2014-06-29 2:24 UTC (permalink / raw) To: Stephan Mueller Cc: Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-29 2:24 ` Stephen Rothwell @ 2014-06-29 3:46 ` Stephan Mueller 2014-06-29 3:53 ` Joe Perches 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-06-29 3:46 UTC (permalink / raw) To: Stephen Rothwell Cc: Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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? | ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-29 3:46 ` Stephan Mueller @ 2014-06-29 3:53 ` Joe Perches 2014-06-29 4:54 ` Stephan Mueller 2014-07-04 11:21 ` Dan Carpenter 0 siblings, 2 replies; 24+ messages in thread From: Joe Perches @ 2014-06-29 3:53 UTC (permalink / raw) To: Stephan Mueller Cc: Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-29 3:53 ` Joe Perches @ 2014-06-29 4:54 ` Stephan Mueller 2014-07-04 11:21 ` Dan Carpenter 1 sibling, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-06-29 4:54 UTC (permalink / raw) To: Joe Perches Cc: Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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? | ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-06-29 3:53 ` Joe Perches @ 2014-07-04 11:21 ` Dan Carpenter 2014-07-04 11:21 ` Dan Carpenter 1 sibling, 0 replies; 24+ messages in thread From: Dan Carpenter @ 2014-07-04 11:21 UTC (permalink / raw) To: Joe Perches Cc: Stephen Rothwell, Herbert Xu, Stephan Mueller, Randy Dunlap, linux-kernel, linux-next, kbuild, linux-crypto 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements @ 2014-07-04 11:21 ` Dan Carpenter 0 siblings, 0 replies; 24+ messages in thread From: Dan Carpenter @ 2014-07-04 11:21 UTC (permalink / raw) To: Joe Perches Cc: Stephan Mueller, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-04 11:21 ` Dan Carpenter (?) @ 2014-07-04 16:57 ` Joe Perches 2014-07-04 23:57 ` Stephan Mueller -1 siblings, 1 reply; 24+ messages in thread From: Joe Perches @ 2014-07-04 16:57 UTC (permalink / raw) To: Dan Carpenter Cc: Stephan Mueller, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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; } ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-04 16:57 ` Joe Perches @ 2014-07-04 23:57 ` Stephan Mueller 2014-07-05 0:09 ` Joe Perches 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-07-04 23:57 UTC (permalink / raw) To: Joe Perches Cc: Dan Carpenter, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-04 23:57 ` Stephan Mueller @ 2014-07-05 0:09 ` Joe Perches 2014-07-05 0:15 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Joe Perches @ 2014-07-05 0:09 UTC (permalink / raw) To: Stephan Mueller Cc: Dan Carpenter, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-05 0:09 ` Joe Perches @ 2014-07-05 0:15 ` Stephan Mueller 2014-07-05 0:24 ` Joe Perches 0 siblings, 1 reply; 24+ messages in thread From: Stephan Mueller @ 2014-07-05 0:15 UTC (permalink / raw) To: Joe Perches Cc: Dan Carpenter, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-05 0:15 ` Stephan Mueller @ 2014-07-05 0:24 ` Joe Perches 2014-07-05 0:27 ` Stephan Mueller 0 siblings, 1 reply; 24+ messages in thread From: Joe Perches @ 2014-07-05 0:24 UTC (permalink / raw) To: Stephan Mueller Cc: Dan Carpenter, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] DRBG: Fix format string for debugging statements 2014-07-05 0:24 ` Joe Perches @ 2014-07-05 0:27 ` Stephan Mueller 0 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-07-05 0:27 UTC (permalink / raw) To: Joe Perches Cc: Dan Carpenter, Stephen Rothwell, Herbert Xu, kbuild test robot, kbuild, linux-crypto, Randy Dunlap, linux-next, linux-kernel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] DRBG: Call CTR DRBG DF function only once 2014-06-28 19:57 [PATCH 0/4] DRBG: Fixes for sparse tool reports Stephan Mueller ` (2 preceding siblings ...) 2014-06-28 20:01 ` [PATCH 3/4] DRBG: Fix format string for debugging statements Stephan Mueller @ 2014-06-28 20:04 ` Stephan Mueller 3 siblings, 0 replies; 24+ messages in thread From: Stephan Mueller @ 2014-06-28 20:04 UTC (permalink / raw) To: Herbert Xu Cc: kbuild test robot, kbuild, Dan Carpenter, linux-crypto, Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-07-05 0:28 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-28 19:57 [PATCH 0/4] DRBG: Fixes for sparse tool reports Stephan Mueller 2014-06-28 19:58 ` [PATCH 1/4] DRBG: use of kernel linked list Stephan Mueller 2014-07-04 14:11 ` Herbert Xu 2014-06-28 20:00 ` [PATCH 2/4] DRBG: cleanup of preprocessor macros Stephan Mueller 2014-06-29 2:20 ` Stephen Rothwell 2014-06-29 5:07 ` Stephan Mueller 2014-06-29 7:41 ` Randy Dunlap 2014-06-29 11:37 ` Stephan Mueller 2014-07-04 14:15 ` Herbert Xu 2014-07-05 0:03 ` Stephan Mueller 2014-06-28 20:01 ` [PATCH 3/4] DRBG: Fix format string for debugging statements Stephan Mueller 2014-06-29 2:24 ` Stephen Rothwell 2014-06-29 3:46 ` Stephan Mueller 2014-06-29 3:53 ` Joe Perches 2014-06-29 4:54 ` Stephan Mueller 2014-07-04 11:21 ` Dan Carpenter 2014-07-04 11:21 ` Dan Carpenter 2014-07-04 16:57 ` Joe Perches 2014-07-04 23:57 ` Stephan Mueller 2014-07-05 0:09 ` Joe Perches 2014-07-05 0:15 ` Stephan Mueller 2014-07-05 0:24 ` Joe Perches 2014-07-05 0:27 ` Stephan Mueller 2014-06-28 20:04 ` [PATCH 4/4] DRBG: Call CTR DRBG DF function only once Stephan Mueller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.