From patchwork Fri Apr 17 12:54:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephan Mueller X-Patchwork-Id: 6230561 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: X-Original-To: patchwork-linux-crypto@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2F0E29F1C4 for ; Fri, 17 Apr 2015 12:56:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0B0FB2037D for ; Fri, 17 Apr 2015 12:56:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4FDB72038C for ; Fri, 17 Apr 2015 12:56:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752216AbbDQM4v (ORCPT ); Fri, 17 Apr 2015 08:56:51 -0400 Received: from mail.eperm.de ([89.247.134.16]:34156 "EHLO mail.eperm.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751988AbbDQM4v (ORCPT ); Fri, 17 Apr 2015 08:56:51 -0400 Received: from myon.chronox.de (unknown [75.144.245.226]) by mail.eperm.de (Postfix) with ESMTPSA id 4AC972A003B; Fri, 17 Apr 2015 14:56:49 +0200 (CEST) From: Stephan Mueller To: herbert@gondor.apana.org.au Cc: linux-crypto@vger.kernel.org Subject: [PATCH 2/4] crypto: drbg - do not create shadow copy Date: Fri, 17 Apr 2015 14:54:58 +0200 Message-ID: <1700799.nu8dYZU873@myon.chronox.de> User-Agent: KMail/4.14.6 (Linux/3.19.3-200.fc21.x86_64; KDE/4.14.6; x86_64; ; ) In-Reply-To: <1551177.C0RmlOO9iU@myon.chronox.de> References: <1551177.C0RmlOO9iU@myon.chronox.de> MIME-Version: 1.0 Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The creation of a shadow copy is intended to only hold a short term lock. But the drawback is that parallel users have a very similar DRBG state which only differs by a high-resolution time stamp. As the locking is changed to use a long-term lock to avoid such similar DRBG states, the entire creation and maintenance of a shadow copy can be removed. Signed-off-by: Stephan Mueller --- crypto/drbg.c | 122 ++++++---------------------------------------------------- 1 file changed, 11 insertions(+), 111 deletions(-) diff --git a/crypto/drbg.c b/crypto/drbg.c index 8d2944f..c8a083c 100644 --- a/crypto/drbg.c +++ b/crypto/drbg.c @@ -1189,79 +1189,6 @@ err: return ret; } -/* - * Strategy to avoid holding long term locks: generate a shadow copy of DRBG - * and perform all operations on this shadow copy. After finishing, restore - * the updated state of the shadow copy into original drbg state. This way, - * only the read and write operations of the original drbg state must be - * locked - */ -static inline void drbg_copy_drbg(struct drbg_state *src, - struct drbg_state *dst) -{ - if (!src || !dst) - return; - memcpy(dst->V, src->V, drbg_statelen(src)); - memcpy(dst->C, src->C, drbg_statelen(src)); - dst->reseed_ctr = src->reseed_ctr; - dst->seeded = src->seeded; - dst->pr = src->pr; -#ifdef CONFIG_CRYPTO_FIPS - dst->fips_primed = src->fips_primed; - memcpy(dst->prev, src->prev, drbg_blocklen(src)); -#endif - /* - * Not copied: - * scratchpad is initialized drbg_alloc_state; - * priv_data is initialized with call to crypto_init; - * d_ops and core are set outside, as these parameters are const; - * test_data is set outside to prevent it being copied back. - */ -} - -static int drbg_make_shadow(struct drbg_state *drbg, struct drbg_state **shadow) -{ - int ret = -ENOMEM; - struct drbg_state *tmp = NULL; - - tmp = kzalloc(sizeof(struct drbg_state), GFP_KERNEL); - if (!tmp) - return -ENOMEM; - - /* read-only data as they are defined as const, no lock needed */ - tmp->core = drbg->core; - tmp->d_ops = drbg->d_ops; - - ret = drbg_alloc_state(tmp); - if (ret) - goto err; - - spin_lock_bh(&drbg->drbg_lock); - drbg_copy_drbg(drbg, tmp); - /* only make a link to the test buffer, as we only read that data */ - tmp->test_data = drbg->test_data; - spin_unlock_bh(&drbg->drbg_lock); - *shadow = tmp; - return 0; - -err: - kzfree(tmp); - return ret; -} - -static void drbg_restore_shadow(struct drbg_state *drbg, - struct drbg_state **shadow) -{ - struct drbg_state *tmp = *shadow; - - spin_lock_bh(&drbg->drbg_lock); - drbg_copy_drbg(tmp, drbg); - spin_unlock_bh(&drbg->drbg_lock); - drbg_dealloc_state(tmp); - kzfree(tmp); - *shadow = NULL; -} - /************************************************************************* * DRBG interface functions *************************************************************************/ @@ -1287,13 +1214,7 @@ static int drbg_generate(struct drbg_state *drbg, struct drbg_string *addtl) { int len = 0; - struct drbg_state *shadow = NULL; LIST_HEAD(addtllist); - struct drbg_string timestamp; - union { - cycles_t cycles; - unsigned char char_cycles[sizeof(cycles_t)]; - } now; if (0 == buflen || !buf) { pr_devel("DRBG: no output buffer provided\n"); @@ -1304,15 +1225,9 @@ static int drbg_generate(struct drbg_state *drbg, return -EINVAL; } - len = drbg_make_shadow(drbg, &shadow); - if (len) { - pr_devel("DRBG: shadow copy cannot be generated\n"); - return len; - } - /* 9.3.1 step 2 */ len = -EINVAL; - if (buflen > (drbg_max_request_bytes(shadow))) { + if (buflen > (drbg_max_request_bytes(drbg))) { pr_devel("DRBG: requested random numbers too large %u\n", buflen); goto err; @@ -1321,7 +1236,7 @@ static int drbg_generate(struct drbg_state *drbg, /* 9.3.1 step 3 is implicit with the chosen DRBG */ /* 9.3.1 step 4 */ - if (addtl && addtl->len > (drbg_max_addtl(shadow))) { + if (addtl && addtl->len > (drbg_max_addtl(drbg))) { pr_devel("DRBG: additional information string too long %zu\n", addtl->len); goto err; @@ -1332,46 +1247,34 @@ static int drbg_generate(struct drbg_state *drbg, * 9.3.1 step 6 and 9 supplemented by 9.3.2 step c is implemented * here. The spec is a bit convoluted here, we make it simpler. */ - if ((drbg_max_requests(shadow)) < shadow->reseed_ctr) - shadow->seeded = false; + if ((drbg_max_requests(drbg)) < drbg->reseed_ctr) + drbg->seeded = false; /* allocate cipher handle */ - len = shadow->d_ops->crypto_init(shadow); + len = drbg->d_ops->crypto_init(drbg); if (len) goto err; - if (shadow->pr || !shadow->seeded) { + if (drbg->pr || !drbg->seeded) { pr_devel("DRBG: reseeding before generation (prediction " "resistance: %s, state %s)\n", drbg->pr ? "true" : "false", drbg->seeded ? "seeded" : "unseeded"); /* 9.3.1 steps 7.1 through 7.3 */ - len = drbg_seed(shadow, addtl, true); + len = drbg_seed(drbg, addtl, true); if (len) goto err; /* 9.3.1 step 7.4 */ addtl = NULL; } - /* - * Mix the time stamp into the DRBG state if the DRBG is not in - * test mode. If there are two callers invoking the DRBG at the same - * time, i.e. before the first caller merges its shadow state back, - * both callers would obtain the same random number stream without - * changing the state here. - */ - if (!drbg->test_data) { - now.cycles = random_get_entropy(); - drbg_string_fill(×tamp, now.char_cycles, sizeof(cycles_t)); - list_add_tail(×tamp.list, &addtllist); - } if (addtl && 0 < addtl->len) list_add_tail(&addtl->list, &addtllist); /* 9.3.1 step 8 and 10 */ - len = shadow->d_ops->generate(shadow, buf, buflen, &addtllist); + len = drbg->d_ops->generate(drbg, buf, buflen, &addtllist); /* 10.1.1.4 step 6, 10.1.2.5 step 7, 10.2.1.5.2 step 7 */ - shadow->reseed_ctr++; + drbg->reseed_ctr++; if (0 >= len) goto err; @@ -1391,7 +1294,7 @@ static int drbg_generate(struct drbg_state *drbg, * case somebody has a need to implement the test of 11.3.3. */ #if 0 - if (shadow->reseed_ctr && !(shadow->reseed_ctr % 4096)) { + if (drbg->reseed_ctr && !(drbg->reseed_ctr % 4096)) { int err = 0; pr_devel("DRBG: start to perform self test\n"); if (drbg->core->flags & DRBG_HMAC) @@ -1410,8 +1313,6 @@ static int drbg_generate(struct drbg_state *drbg, * are returned when reusing this DRBG cipher handle */ drbg_uninstantiate(drbg); - drbg_dealloc_state(shadow); - kzfree(shadow); return 0; } else { pr_devel("DRBG: self test successful\n"); @@ -1425,8 +1326,7 @@ static int drbg_generate(struct drbg_state *drbg, */ len = 0; err: - shadow->d_ops->crypto_fini(shadow); - drbg_restore_shadow(drbg, &shadow); + drbg->d_ops->crypto_fini(drbg); return len; }