Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-27 Thread H. Peter Anvin

On 07/27/2012 07:39 PM, Theodore Ts'o wrote:

Ok, I'll add this patch to the random tree.  I've modified the commit
message a bit since the speed advertisement of RDRAND is rather
pointless --- processes aren't generating session keys or long term
keys at a high rate, and programs can't count on /dev/random being
super fast and having unlimited entropy, since for most platforms and
even most x86 CPU's deployed in service today, this isn't true --- and
making your userspace program depond upon /dev/random in such a way
that it only works on Ivy Bridge CPU's might be good for Intel from a
vendor lock-in perspective, but it's really bad, non-portable
programming style.

Also, in the future arch_get_random_long() will almost certainly be
hooked up for other architectures, so putting an extended
advertisement for RDRAND really isn't appropriate.


Thanks.  /dev/random vs /dev/urandom is orthogonal to this; as you note 
we still haven't changed the entropy accounting.  I am thinking that 
that is probably better left to rngd at least until RDSEED is available 
(or the equivalent on other hardware.)


-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

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


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-27 Thread Theodore Ts'o
Ok, I'll add this patch to the random tree.  I've modified the commit
message a bit since the speed advertisement of RDRAND is rather
pointless --- processes aren't generating session keys or long term
keys at a high rate, and programs can't count on /dev/random being
super fast and having unlimited entropy, since for most platforms and
even most x86 CPU's deployed in service today, this isn't true --- and
making your userspace program depond upon /dev/random in such a way
that it only works on Ivy Bridge CPU's might be good for Intel from a
vendor lock-in perspective, but it's really bad, non-portable
programming style.

Also, in the future arch_get_random_long() will almost certainly be
hooked up for other architectures, so putting an extended
advertisement for RDRAND really isn't appropriate.

- Ted

commit d2e7c96af1e54b507ae2a6a7dd2baf588417a7e5
Author: H. Peter Anvin 
Date:   Fri Jul 27 22:26:08 2012 -0400

random: mix in architectural randomness in extract_buf()

Mix in any architectural randomness in extract_buf() instead of
xfer_secondary_buf().  This allows us to mix in more architectural
randomness, and it also makes xfer_secondary_buf() faster, moving a
tiny bit of additional CPU overhead to process which is extracting the
randomness.

[ Commit description modified by tytso to remove an extended
  advertisement for the RDRAND instruction. ]

Signed-off-by: H. Peter Anvin 
Acked-by: Ingo Molnar 
Cc: DJ Johnston 
Signed-off-by: Theodore Ts'o 
Cc: sta...@vger.kernel.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-27 Thread Theodore Ts'o
Ok, I'll add this patch to the random tree.  I've modified the commit
message a bit since the speed advertisement of RDRAND is rather
pointless --- processes aren't generating session keys or long term
keys at a high rate, and programs can't count on /dev/random being
super fast and having unlimited entropy, since for most platforms and
even most x86 CPU's deployed in service today, this isn't true --- and
making your userspace program depond upon /dev/random in such a way
that it only works on Ivy Bridge CPU's might be good for Intel from a
vendor lock-in perspective, but it's really bad, non-portable
programming style.

Also, in the future arch_get_random_long() will almost certainly be
hooked up for other architectures, so putting an extended
advertisement for RDRAND really isn't appropriate.

- Ted

commit d2e7c96af1e54b507ae2a6a7dd2baf588417a7e5
Author: H. Peter Anvin h...@linux.intel.com
Date:   Fri Jul 27 22:26:08 2012 -0400

random: mix in architectural randomness in extract_buf()

Mix in any architectural randomness in extract_buf() instead of
xfer_secondary_buf().  This allows us to mix in more architectural
randomness, and it also makes xfer_secondary_buf() faster, moving a
tiny bit of additional CPU overhead to process which is extracting the
randomness.

[ Commit description modified by tytso to remove an extended
  advertisement for the RDRAND instruction. ]

Signed-off-by: H. Peter Anvin h...@linux.intel.com
Acked-by: Ingo Molnar mi...@kernel.org
Cc: DJ Johnston dj.johns...@intel.com
Signed-off-by: Theodore Ts'o ty...@mit.edu
Cc: sta...@vger.kernel.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-27 Thread H. Peter Anvin

On 07/27/2012 07:39 PM, Theodore Ts'o wrote:

Ok, I'll add this patch to the random tree.  I've modified the commit
message a bit since the speed advertisement of RDRAND is rather
pointless --- processes aren't generating session keys or long term
keys at a high rate, and programs can't count on /dev/random being
super fast and having unlimited entropy, since for most platforms and
even most x86 CPU's deployed in service today, this isn't true --- and
making your userspace program depond upon /dev/random in such a way
that it only works on Ivy Bridge CPU's might be good for Intel from a
vendor lock-in perspective, but it's really bad, non-portable
programming style.

Also, in the future arch_get_random_long() will almost certainly be
hooked up for other architectures, so putting an extended
advertisement for RDRAND really isn't appropriate.


Thanks.  /dev/random vs /dev/urandom is orthogonal to this; as you note 
we still haven't changed the entropy accounting.  I am thinking that 
that is probably better left to rngd at least until RDSEED is available 
(or the equivalent on other hardware.)


-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread H. Peter Anvin
On 07/25/2012 04:50 PM, Ben Hutchings wrote:
> On Wed, 2012-07-25 at 10:37 -0700, H. Peter Anvin wrote:
>> From: "H. Peter Anvin" 
>>
>> RDRAND is so much faster than the Linux pool system that we can
>> always just mix in architectural randomness.
> [...]
>> Signed-off-by: H. Peter Anvin 
>> Acked-by: Ingo Molnar 
>> Cc: DJ Johnston 
> [...]
> 
> This is not the correct way to submit patches for stable; see
> Documentation/stable_kernel_rules.txt
> 

The patch is intended to fix the security regression that would be
caused if Ted's random.git patches are accepted in -stable, which isn't
clear to me if they will be or not, so I added the Cc: to keep it from
getting lost.

-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread Ben Hutchings
On Wed, 2012-07-25 at 10:37 -0700, H. Peter Anvin wrote:
> From: "H. Peter Anvin" 
> 
> RDRAND is so much faster than the Linux pool system that we can
> always just mix in architectural randomness.
[...]
> Signed-off-by: H. Peter Anvin 
> Acked-by: Ingo Molnar 
> Cc: DJ Johnston 
[...]

This is not the correct way to submit patches for stable; see
Documentation/stable_kernel_rules.txt

Ben.
(needs to get one of those form letters like Greg)

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.


signature.asc
Description: This is a digitally signed message part


[PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread H. Peter Anvin
From: "H. Peter Anvin" 

RDRAND is so much faster than the Linux pool system that we can
always just mix in architectural randomness.

Doing this in extract_buf() lets us do this in one convenient
place, unfortunately the output size (10 bytes) is maximally
awkward.  That, plus the fact that every output byte will have
passed through extract_buf() twice means we are not being very
efficient with the RDRAND use.

Measurements show that RDRAND is 12-15 times faster than the Linux
pool system.  Doing the math shows this corresponds to about an
11.5% slowdown which is confirmed by measurements.

Users who are very performance- or power-sensitive could definitely
still benefit from being allowed to use RDRAND directly, but I
believe this version should satisfy even the most hyper-paranoid
crowd.

[ v2: changed the final memcpy() from hash.w to hash in order to
  make it more obvious that mucking with hash.l doesn't leave
  hash.w unmodified.  This doesn't have any impact on the final
  code as the Linux kernel is alwasy compiled with -fno-strict-aliasing
  but it might make it more obvious to the human reader who might get
  confused and think it is a structure. ]

Signed-off-by: H. Peter Anvin 
Acked-by: Ingo Molnar 
Cc: DJ Johnston 
---
 drivers/char/random.c | 56 +--
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9793b40..7bc0b36 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -277,6 +277,8 @@
 #define SEC_XFER_SIZE 512
 #define EXTRACT_SIZE 10
 
+#define LONGS(x) (((x) + sizeof(unsigned long) - 1)/sizeof(unsigned long))
+
 /*
  * The minimum number of bits of entropy before we wake up a read on
  * /dev/random.  Should be enough to do a significant reseed.
@@ -813,11 +815,7 @@ static ssize_t extract_entropy(struct entropy_store *r, 
void *buf,
  */
 static void xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 {
-   union {
-   __u32   tmp[OUTPUT_POOL_WORDS];
-   longhwrand[4];
-   } u;
-   int i;
+   __u32   tmp[OUTPUT_POOL_WORDS];
 
if (r->pull && r->entropy_count < nbytes * 8 &&
r->entropy_count < r->poolinfo->POOLBITS) {
@@ -828,23 +826,17 @@ static void xfer_secondary_pool(struct entropy_store *r, 
size_t nbytes)
/* pull at least as many as BYTES as wakeup BITS */
bytes = max_t(int, bytes, random_read_wakeup_thresh / 8);
/* but never more than the buffer size */
-   bytes = min_t(int, bytes, sizeof(u.tmp));
+   bytes = min_t(int, bytes, sizeof(tmp));
 
DEBUG_ENT("going to reseed %s with %d bits "
  "(%d of %d requested)\n",
  r->name, bytes * 8, nbytes * 8, r->entropy_count);
 
-   bytes = extract_entropy(r->pull, u.tmp, bytes,
+   bytes = extract_entropy(r->pull, tmp, bytes,
random_read_wakeup_thresh / 8, rsvd);
-   mix_pool_bytes(r, u.tmp, bytes, NULL);
+   mix_pool_bytes(r, tmp, bytes, NULL);
credit_entropy_bits(r, bytes*8);
}
-   kmemcheck_mark_initialized(, sizeof(u.hwrand));
-   for (i = 0; i < 4; i++)
-   if (arch_get_random_long([i]))
-   break;
-   if (i)
-   mix_pool_bytes(r, , sizeof(u.hwrand), 0);
 }
 
 /*
@@ -901,15 +893,19 @@ static size_t account(struct entropy_store *r, size_t 
nbytes, int min,
 static void extract_buf(struct entropy_store *r, __u8 *out)
 {
int i;
-   __u32 hash[5], workspace[SHA_WORKSPACE_WORDS];
+   union {
+   __u32 w[5];
+   unsigned long l[LONGS(EXTRACT_SIZE)];
+   } hash;
+   __u32 workspace[SHA_WORKSPACE_WORDS];
__u8 extract[64];
unsigned long flags;
 
/* Generate a hash across the pool, 16 words (512 bits) at a time */
-   sha_init(hash);
+   sha_init(hash.w);
spin_lock_irqsave(>lock, flags);
for (i = 0; i < r->poolinfo->poolwords; i += 16)
-   sha_transform(hash, (__u8 *)(r->pool + i), workspace);
+   sha_transform(hash.w, (__u8 *)(r->pool + i), workspace);
 
/*
 * We mix the hash back into the pool to prevent backtracking
@@ -920,14 +916,14 @@ static void extract_buf(struct entropy_store *r, __u8 
*out)
 * brute-forcing the feedback as hard as brute-forcing the
 * hash.
 */
-   __mix_pool_bytes(r, hash, sizeof(hash), extract);
+   __mix_pool_bytes(r, hash.w, sizeof(hash.w), extract);
spin_unlock_irqrestore(>lock, flags);
 
/*
 * To avoid duplicates, we atomically extract a portion of the
 * pool while mixing, and hash one final time.
 */
-   sha_transform(hash, extract, workspace);
+   sha_transform(hash.w, extract, workspace);

[PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread H. Peter Anvin
From: H. Peter Anvin h...@linux.intel.com

RDRAND is so much faster than the Linux pool system that we can
always just mix in architectural randomness.

Doing this in extract_buf() lets us do this in one convenient
place, unfortunately the output size (10 bytes) is maximally
awkward.  That, plus the fact that every output byte will have
passed through extract_buf() twice means we are not being very
efficient with the RDRAND use.

Measurements show that RDRAND is 12-15 times faster than the Linux
pool system.  Doing the math shows this corresponds to about an
11.5% slowdown which is confirmed by measurements.

Users who are very performance- or power-sensitive could definitely
still benefit from being allowed to use RDRAND directly, but I
believe this version should satisfy even the most hyper-paranoid
crowd.

[ v2: changed the final memcpy() from hash.w to hash in order to
  make it more obvious that mucking with hash.l doesn't leave
  hash.w unmodified.  This doesn't have any impact on the final
  code as the Linux kernel is alwasy compiled with -fno-strict-aliasing
  but it might make it more obvious to the human reader who might get
  confused and think it is a structure. ]

Signed-off-by: H. Peter Anvin h...@linux.intel.com
Acked-by: Ingo Molnar mi...@kernel.org
Cc: DJ Johnston dj.johns...@intel.com
---
 drivers/char/random.c | 56 +--
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 9793b40..7bc0b36 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -277,6 +277,8 @@
 #define SEC_XFER_SIZE 512
 #define EXTRACT_SIZE 10
 
+#define LONGS(x) (((x) + sizeof(unsigned long) - 1)/sizeof(unsigned long))
+
 /*
  * The minimum number of bits of entropy before we wake up a read on
  * /dev/random.  Should be enough to do a significant reseed.
@@ -813,11 +815,7 @@ static ssize_t extract_entropy(struct entropy_store *r, 
void *buf,
  */
 static void xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 {
-   union {
-   __u32   tmp[OUTPUT_POOL_WORDS];
-   longhwrand[4];
-   } u;
-   int i;
+   __u32   tmp[OUTPUT_POOL_WORDS];
 
if (r-pull  r-entropy_count  nbytes * 8 
r-entropy_count  r-poolinfo-POOLBITS) {
@@ -828,23 +826,17 @@ static void xfer_secondary_pool(struct entropy_store *r, 
size_t nbytes)
/* pull at least as many as BYTES as wakeup BITS */
bytes = max_t(int, bytes, random_read_wakeup_thresh / 8);
/* but never more than the buffer size */
-   bytes = min_t(int, bytes, sizeof(u.tmp));
+   bytes = min_t(int, bytes, sizeof(tmp));
 
DEBUG_ENT(going to reseed %s with %d bits 
  (%d of %d requested)\n,
  r-name, bytes * 8, nbytes * 8, r-entropy_count);
 
-   bytes = extract_entropy(r-pull, u.tmp, bytes,
+   bytes = extract_entropy(r-pull, tmp, bytes,
random_read_wakeup_thresh / 8, rsvd);
-   mix_pool_bytes(r, u.tmp, bytes, NULL);
+   mix_pool_bytes(r, tmp, bytes, NULL);
credit_entropy_bits(r, bytes*8);
}
-   kmemcheck_mark_initialized(u.hwrand, sizeof(u.hwrand));
-   for (i = 0; i  4; i++)
-   if (arch_get_random_long(u.hwrand[i]))
-   break;
-   if (i)
-   mix_pool_bytes(r, u.hwrand, sizeof(u.hwrand), 0);
 }
 
 /*
@@ -901,15 +893,19 @@ static size_t account(struct entropy_store *r, size_t 
nbytes, int min,
 static void extract_buf(struct entropy_store *r, __u8 *out)
 {
int i;
-   __u32 hash[5], workspace[SHA_WORKSPACE_WORDS];
+   union {
+   __u32 w[5];
+   unsigned long l[LONGS(EXTRACT_SIZE)];
+   } hash;
+   __u32 workspace[SHA_WORKSPACE_WORDS];
__u8 extract[64];
unsigned long flags;
 
/* Generate a hash across the pool, 16 words (512 bits) at a time */
-   sha_init(hash);
+   sha_init(hash.w);
spin_lock_irqsave(r-lock, flags);
for (i = 0; i  r-poolinfo-poolwords; i += 16)
-   sha_transform(hash, (__u8 *)(r-pool + i), workspace);
+   sha_transform(hash.w, (__u8 *)(r-pool + i), workspace);
 
/*
 * We mix the hash back into the pool to prevent backtracking
@@ -920,14 +916,14 @@ static void extract_buf(struct entropy_store *r, __u8 
*out)
 * brute-forcing the feedback as hard as brute-forcing the
 * hash.
 */
-   __mix_pool_bytes(r, hash, sizeof(hash), extract);
+   __mix_pool_bytes(r, hash.w, sizeof(hash.w), extract);
spin_unlock_irqrestore(r-lock, flags);
 
/*
 * To avoid duplicates, we atomically extract a portion of the
 * pool while mixing, and hash one final time.
 */
-   sha_transform(hash, 

Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread Ben Hutchings
On Wed, 2012-07-25 at 10:37 -0700, H. Peter Anvin wrote:
 From: H. Peter Anvin h...@linux.intel.com
 
 RDRAND is so much faster than the Linux pool system that we can
 always just mix in architectural randomness.
[...]
 Signed-off-by: H. Peter Anvin h...@linux.intel.com
 Acked-by: Ingo Molnar mi...@kernel.org
 Cc: DJ Johnston dj.johns...@intel.com
[...]

This is not the correct way to submit patches for stable; see
Documentation/stable_kernel_rules.txt

Ben.
(needs to get one of those form letters like Greg)

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.


signature.asc
Description: This is a digitally signed message part


Re: [PATCH] random: mix in architectural randomness in extract_buf()

2012-07-25 Thread H. Peter Anvin
On 07/25/2012 04:50 PM, Ben Hutchings wrote:
 On Wed, 2012-07-25 at 10:37 -0700, H. Peter Anvin wrote:
 From: H. Peter Anvin h...@linux.intel.com

 RDRAND is so much faster than the Linux pool system that we can
 always just mix in architectural randomness.
 [...]
 Signed-off-by: H. Peter Anvin h...@linux.intel.com
 Acked-by: Ingo Molnar mi...@kernel.org
 Cc: DJ Johnston dj.johns...@intel.com
 [...]
 
 This is not the correct way to submit patches for stable; see
 Documentation/stable_kernel_rules.txt
 

The patch is intended to fix the security regression that would be
caused if Ted's random.git patches are accepted in -stable, which isn't
clear to me if they will be or not, so I added the Cc: to keep it from
getting lost.

-hpa
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/