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 <smuel...@chronox.de>
---
 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(&timestamp, now.char_cycles, sizeof(cycles_t));
-               list_add_tail(&timestamp.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;
 }
 
-- 
2.1.0


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to