linux-kernel - [PATCH 02/14] random: fix a (harmless) overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131215020057.GB27191@athena.dialup.mit.edu>
Date:	Sat, 14 Dec 2013 21:00:57 -0500
From:	Greg Price <price@....EDU>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	linux-kernel@...r.kernel.org
Subject: [PATCH 02/14] random: fix a (harmless) overflow

This overflow is harmless except to think about, but it's best
to fix it.  If userspace does a giant read from /dev/urandom,
bigger than INT_MAX, then that size gets passed straight
through extract_entropy_user and xfer_secondary_pool to
_xfer_secondary_pool as nbytes, and we would store it into
bytes, which is an int.  The result could be negative.

The consequence is pretty small -- we would pull only the minimum
amount of entropy, rather than as much as we could up to the size
of the output pool, and this is urandom so that's fine.  But the
code is a little easier to read if we make it clear that overflow
isn't an issue.  Also we might be less likely to make mistakes like
the one fixed in the previous commit.

As a bonus, give a name to the minimum number of bytes to pull,
which we use twice.

Signed-off-by: Greg Price <price@....edu>
---
 drivers/char/random.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 1dd5f2634..92d9f6862 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -922,21 +922,20 @@ static void xfer_secondary_pool(struct entropy_store *r, size_t nbytes)

 static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes)
 {
-	__u32	tmp[OUTPUT_POOL_WORDS];
+	__u32 tmp[OUTPUT_POOL_WORDS];
+	int bytes, min_bytes;

 	/* For /dev/random's pool, always leave two wakeups' worth */
 	int rsvd_bytes = r->limit ? 0 : random_read_wakeup_bits / 4;
-	int bytes = nbytes;

 	/* pull at least as much as a wakeup */
-	bytes = max_t(int, bytes, random_read_wakeup_bits / 8);
+	min_bytes = random_read_wakeup_bits / 8;
 	/* but never more than the buffer size */
-	bytes = min_t(int, bytes, sizeof(tmp));
+	bytes = min(sizeof(tmp), max_t(size_t, min_bytes, nbytes));

 	trace_xfer_secondary_pool(r->name, bytes * 8, nbytes * 8,
 				  ENTROPY_BITS(r), ENTROPY_BITS(r->pull));
-	bytes = extract_entropy(r->pull, tmp, bytes,
-				random_read_wakeup_bits / 8, rsvd_bytes);
+	bytes = extract_entropy(r->pull, tmp, bytes, min_bytes, rsvd_bytes);
 	mix_pool_bytes(r, tmp, bytes, NULL);
 	credit_entropy_bits(r, bytes*8);
 }
-- 
1.8.3.2

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