From: Sebastian Andrzej Siewior Subject: Re: [PATCH]: fix repetition test for hardware RNG to be FIPS compliant Date: Sun, 13 Sep 2009 14:17:34 +0200 Message-ID: <20090913121734.GA23157@Chamillionaire.breakpoint.cc> References: <20090912164411.GA4735@localhost.localdomain> Reply-To: Sebastian Andrzej Siewior Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Cc: linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au, davem@davemloft.net To: Neil Horman Return-path: Received: from Chamillionaire.breakpoint.cc ([85.10.199.196]:37700 "EHLO Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753374AbZIMMRf (ORCPT ); Sun, 13 Sep 2009 08:17:35 -0400 Content-Disposition: inline In-Reply-To: <20090912164411.GA4735@localhost.localdomain> Sender: linux-crypto-owner@vger.kernel.org List-ID: * Neil Horman | 2009-09-12 12:44:11 [-0400]: >diff --git a/drivers/char/random.c b/drivers/char/random.c >index d8a9255..6700248 100644 >--- a/drivers/char/random.c >+++ b/drivers/char/random.c >@@ -399,6 +399,12 @@ module_param(debug, bool, 0644); > * storing entropy in an entropy pool. > * > **********************************************************************/ >+#define EXTRACT_SIZE 10 >+#define REP_CHECK_BLOCK_COPIED 1 >+struct repetition_check { >+ __u8 last_data[EXTRACT_SIZE]; >+ __u8 flags; >+}; This struct should have 11 bytes and is only used in FIPS enabled mode. > struct entropy_store; > struct entropy_store { >@@ -414,7 +420,7 @@ struct entropy_store { > unsigned add_ptr; > int entropy_count; > int input_rotate; >- __u8 *last_data; >+ struct repetition_check *rep; > }; so just a pointer to 11 bytes > static __u32 input_pool_data[INPUT_POOL_WORDS]; >@@ -714,7 +720,6 @@ void ad > >-#define EXTRACT_SIZE 10 > > /********************************************************************* > * >@@ -856,18 +861,24 @@ static ssize_t extract_entropy(struct entropy_store *r, void *buf, > __u8 tmp[EXTRACT_SIZE]; > unsigned long flags; > >+repeat_extract: > xfer_secondary_pool(r, nbytes); > nbytes = account(r, nbytes, min, reserved); > > while (nbytes) { > extract_buf(r, tmp); > >- if (r->last_data) { >+ if (r->rep) { > spin_lock_irqsave(&r->lock, flags); >- if (!memcmp(tmp, r->last_data, EXTRACT_SIZE)) >+ if ((r->rep->flags & REP_CHECK_BLOCK_COPIED) && >+ !memcmp(tmp, r->rep->last_data, EXTRACT_SIZE)) > panic("Hardware RNG duplicated output!\n"); >- memcpy(r->last_data, tmp, EXTRACT_SIZE); >+ memcpy(r->rep->last_data, tmp, EXTRACT_SIZE); > spin_unlock_irqrestore(&r->lock, flags); >+ if (!(r->rep->flags & REP_CHECK_BLOCK_COPIED)) { >+ r->rep->flags |= REP_CHECK_BLOCK_COPIED; >+ goto repeat_extract; >+ } > } > i = min_t(int, nbytes, EXTRACT_SIZE); > memcpy(buf, tmp, i); >@@ -952,8 +963,10 @@ static void init_std_data(struct entropy_store *r) > mix_pool_bytes(r, &now, sizeof(now)); > mix_pool_bytes(r, utsname(), sizeof(*(utsname()))); > /* Enable continuous test in fips mode */ >- if (fips_enabled) >- r->last_data = kmalloc(EXTRACT_SIZE, GFP_KERNEL); >+ if (fips_enabled) { >+ r->rep = kmalloc(sizeof(struct repetition_check), GFP_KERNEL); and we alloc this 11 bytes via kmalloc. The smallest allocation is afaik 32 bytes. The three pools (input_pool, blocking_pool, nonblocking_pool) are in data seg so it probably does not hurt if you add the extra 10 bytes there. The advantage: - you don't have to deal with -ENOMEM. Otherwise it could be possible that you not doing anything special in fips_mode - you go OOM if the user is using RNDCLEARPOOL ioctl(). Slowly but you do. >+ r->rep->flags = 0; >+ } > } > > static int rand_initialize(void) Sebastian