linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
@ 2012-10-28  7:18 Akinobu Mita
  2012-10-28  7:18 ` [PATCH 2/9] uuid: use random32_get_bytes() Akinobu Mita
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:18 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Theodore Ts'o, Artem Bityutskiy, Adrian Hunter,
	David Woodhouse, linux-mtd

Add functions to get the requested number of pseudo-random bytes.

The difference from get_random_bytes() is that it uses pseudo-random
numbers generated by random32.  It is fast, suitable for generating
random bytes for testing, and reproducible if prandom32 interface is used.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 include/linux/random.h |  2 ++
 lib/random32.c         | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/linux/random.h b/include/linux/random.h
index 6330ed4..eedf429b 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -27,8 +27,10 @@ unsigned long randomize_range(unsigned long start, unsigned long end, unsigned l
 
 u32 random32(void);
 void srandom32(u32 seed);
+void random32_get_bytes(void *buf, int nbytes);
 
 u32 prandom32(struct rnd_state *);
+void prandom32_get_bytes(struct rnd_state *state, void *buf, int nbytes);
 
 /*
  * Handle minimum values for seeds
diff --git a/lib/random32.c b/lib/random32.c
index 938bde5..aee87dd 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -61,6 +61,44 @@ u32 prandom32(struct rnd_state *state)
 EXPORT_SYMBOL(prandom32);
 
 /**
+ *	prandom32_get_bytes - get the requested number of pseudo-random bytes
+ *	@state: pointer to state structure holding seeded state.
+ *	@buf: where to copy the pseudo-random bytes to
+ *	@bytes: the requested number of bytes
+ *
+ *	This is used for pseudo-randomness with no outside seeding.
+ *	For more random results, use random32_get_bytes().
+ */
+void prandom32_get_bytes(struct rnd_state *state, void *buf, int bytes)
+{
+	unsigned char *p = buf;
+
+	for (; bytes > 0 && ((unsigned long)p) % sizeof(u32); bytes--, p++)
+		*p = prandom32(state);
+
+	for (; bytes > sizeof(u32) - 1; bytes -= sizeof(u32), p += sizeof(u32))
+		*(u32 *)p = prandom32(state);
+
+	for (; bytes > 0; bytes--, p++)
+		*p = prandom32(state);
+}
+EXPORT_SYMBOL(prandom32_get_bytes);
+
+/**
+ *	random32_get_bytes - get the requested number of pseudo-random bytes
+ *	@buf: where to copy the pseudo-random bytes to
+ *	@bytes: the requested number of bytes
+ */
+void random32_get_bytes(void *buf, int bytes)
+{
+	struct rnd_state *state = &get_cpu_var(net_rand_state);
+
+	prandom32_get_bytes(state, buf, bytes);
+	put_cpu_var(state);
+}
+EXPORT_SYMBOL(random32_get_bytes);
+
+/**
  *	random32 - pseudo random number generator
  *
  *	A 32 bit pseudo-random number is generated using a fast
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
@ 2012-10-28  7:18 ` Akinobu Mita
  2012-10-29 20:52   ` Theodore Ts'o
  2012-10-28  7:19 ` [PATCH 3/9] mtd: mtd_nandecctest: use random32_get_bytes instead of get_random_bytes() Akinobu Mita
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:18 UTC (permalink / raw)
  To: linux-kernel, akpm; +Cc: Akinobu Mita

Use random32_get_bytes() to generate 16 bytes of pseudo-random bytes.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 lib/uuid.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/lib/uuid.c b/lib/uuid.c
index 52a6fe6..697f867 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -25,13 +25,7 @@
 
 static void __uuid_gen_common(__u8 b[16])
 {
-	int i;
-	u32 r;
-
-	for (i = 0; i < 4; i++) {
-		r = random32();
-		memcpy(b + i * 4, &r, 4);
-	}
+	random32_get_bytes(b, 16);
 	/* reversion 0b10 */
 	b[8] = (b[8] & 0x3F) | 0x80;
 }
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/9] mtd: mtd_nandecctest: use random32_get_bytes instead of get_random_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
  2012-10-28  7:18 ` [PATCH 2/9] uuid: use random32_get_bytes() Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 4/9] mtd: nandsim: use random32_get_bytes Akinobu Mita
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

Using random32_get_bytes() is enough.  Because this data is only used
for testing, not used for cryptographic use.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/tests/mtd_nandecctest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index a5fb9b0..0ddcbec 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -253,7 +253,7 @@ static int nand_ecc_test_run(const size_t size)
 		goto error;
 	}
 
-	get_random_bytes(correct_data, size);
+	random32_get_bytes(correct_data, size);
 	__nand_calculate_ecc(correct_data, size, correct_ecc);
 
 	for (i = 0; i < ARRAY_SIZE(nand_ecc_test); i++) {
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/9] mtd: nandsim: use random32_get_bytes
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
  2012-10-28  7:18 ` [PATCH 2/9] uuid: use random32_get_bytes() Akinobu Mita
  2012-10-28  7:19 ` [PATCH 3/9] mtd: mtd_nandecctest: use random32_get_bytes instead of get_random_bytes() Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 5/9] ubifs: " Akinobu Mita
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

This also removes unnecessary memset call which is immediately overwritten
with random bytes.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/nand/nandsim.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index a932c48..d074b4c 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -1397,10 +1397,7 @@ int do_read_error(struct nandsim *ns, int num)
 	unsigned int page_no = ns->regs.row;
 
 	if (read_error(page_no)) {
-		int i;
-		memset(ns->buf.byte, 0xFF, num);
-		for (i = 0; i < num; ++i)
-			ns->buf.byte[i] = random32();
+		random32_get_bytes(ns->buf.byte, num);
 		NS_WARN("simulating read error in page %u\n", page_no);
 		return 1;
 	}
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/9] ubifs: use random32_get_bytes
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (2 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 4/9] mtd: nandsim: use random32_get_bytes Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 6/9] mtd: mtd_oobtest: convert to use prandom32_get_bytes() Akinobu Mita
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, Adrian Hunter, linux-mtd

This also converts filling memory loop to use memset.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: linux-mtd@lists.infradead.org
---
 fs/ubifs/debug.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 6291163..f3e4a25 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2560,7 +2560,7 @@ static int power_cut_emulated(struct ubifs_info *c, int lnum, int write)
 static int corrupt_data(const struct ubifs_info *c, const void *buf,
 			unsigned int len)
 {
-	unsigned int from, to, i, ffs = chance(1, 2);
+	unsigned int from, to, ffs = chance(1, 2);
 	unsigned char *p = (void *)buf;
 
 	from = random32() % (len + 1);
@@ -2571,11 +2571,9 @@ static int corrupt_data(const struct ubifs_info *c, const void *buf,
 		   ffs ? "0xFFs" : "random data");
 
 	if (ffs)
-		for (i = from; i < to; i++)
-			p[i] = 0xFF;
+		memset(p + from, 0xFF, to - from);
 	else
-		for (i = from; i < to; i++)
-			p[i] = random32() % 0x100;
+		random32_get_bytes(p + from, to - from);
 
 	return to;
 }
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/9] mtd: mtd_oobtest: convert to use prandom32_get_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (3 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 5/9] ubifs: " Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 7/9] mtd: mtd_pagetest: " Akinobu Mita
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

This removes home-brewed pseudo-random number generator and use
prandom32_get_bytes().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/tests/mtd_oobtest.c | 49 ++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/mtd/tests/mtd_oobtest.c b/drivers/mtd/tests/mtd_oobtest.c
index ed9b628..41670ed 100644
--- a/drivers/mtd/tests/mtd_oobtest.c
+++ b/drivers/mtd/tests/mtd_oobtest.c
@@ -27,6 +27,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/random.h>
 
 #define PRINT_PREF KERN_INFO "mtd_oobtest: "
 
@@ -46,26 +47,7 @@ static int use_offset;
 static int use_len;
 static int use_len_max;
 static int vary_offset;
-static unsigned long next = 1;
-
-static inline unsigned int simple_rand(void)
-{
-	next = next * 1103515245 + 12345;
-	return (unsigned int)((next / 65536) % 32768);
-}
-
-static inline void simple_srand(unsigned long seed)
-{
-	next = seed;
-}
-
-static void set_random_data(unsigned char *buf, size_t len)
-{
-	size_t i;
-
-	for (i = 0; i < len; ++i)
-		buf[i] = simple_rand();
-}
+static struct rnd_state rnd_state;
 
 static int erase_eraseblock(int ebnum)
 {
@@ -130,7 +112,7 @@ static int write_eraseblock(int ebnum)
 	loff_t addr = ebnum * mtd->erasesize;
 
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
-		set_random_data(writebuf, use_len);
+		prandom32_get_bytes(&rnd_state, writebuf, use_len);
 		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
@@ -183,7 +165,7 @@ static int verify_eraseblock(int ebnum)
 	loff_t addr = ebnum * mtd->erasesize;
 
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
-		set_random_data(writebuf, use_len);
+		prandom32_get_bytes(&rnd_state, writebuf, use_len);
 		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
 		ops.retlen    = 0;
@@ -275,7 +257,7 @@ static int verify_eraseblock_in_one_go(int ebnum)
 	loff_t addr = ebnum * mtd->erasesize;
 	size_t len = mtd->ecclayout->oobavail * pgcnt;
 
-	set_random_data(writebuf, len);
+	prandom32_get_bytes(&rnd_state, writebuf, len);
 	ops.mode      = MTD_OPS_AUTO_OOB;
 	ops.len       = 0;
 	ops.retlen    = 0;
@@ -426,12 +408,12 @@ static int __init mtd_oobtest_init(void)
 	if (err)
 		goto out;
 
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	err = write_whole_device();
 	if (err)
 		goto out;
 
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	err = verify_all_eraseblocks();
 	if (err)
 		goto out;
@@ -446,13 +428,13 @@ static int __init mtd_oobtest_init(void)
 	if (err)
 		goto out;
 
-	simple_srand(3);
+	prandom32_seed(&rnd_state, 3);
 	err = write_whole_device();
 	if (err)
 		goto out;
 
 	/* Check all eraseblocks */
-	simple_srand(3);
+	prandom32_seed(&rnd_state, 3);
 	printk(PRINT_PREF "verifying all eraseblocks\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
@@ -481,7 +463,7 @@ static int __init mtd_oobtest_init(void)
 	use_len = mtd->ecclayout->oobavail;
 	use_len_max = mtd->ecclayout->oobavail;
 	vary_offset = 1;
-	simple_srand(5);
+	prandom32_seed(&rnd_state, 5);
 
 	err = write_whole_device();
 	if (err)
@@ -492,7 +474,7 @@ static int __init mtd_oobtest_init(void)
 	use_len = mtd->ecclayout->oobavail;
 	use_len_max = mtd->ecclayout->oobavail;
 	vary_offset = 1;
-	simple_srand(5);
+	prandom32_seed(&rnd_state, 5);
 	err = verify_all_eraseblocks();
 	if (err)
 		goto out;
@@ -651,7 +633,7 @@ static int __init mtd_oobtest_init(void)
 		goto out;
 
 	/* Write all eraseblocks */
-	simple_srand(11);
+	prandom32_seed(&rnd_state, 11);
 	printk(PRINT_PREF "writing OOBs of whole device\n");
 	for (i = 0; i < ebcnt - 1; ++i) {
 		int cnt = 2;
@@ -661,7 +643,7 @@ static int __init mtd_oobtest_init(void)
 			continue;
 		addr = (i + 1) * mtd->erasesize - mtd->writesize;
 		for (pg = 0; pg < cnt; ++pg) {
-			set_random_data(writebuf, sz);
+			prandom32_get_bytes(&rnd_state, writebuf, sz);
 			ops.mode      = MTD_OPS_AUTO_OOB;
 			ops.len       = 0;
 			ops.retlen    = 0;
@@ -683,12 +665,13 @@ static int __init mtd_oobtest_init(void)
 	printk(PRINT_PREF "written %u eraseblocks\n", i);
 
 	/* Check all eraseblocks */
-	simple_srand(11);
+	prandom32_seed(&rnd_state, 11);
 	printk(PRINT_PREF "verifying all eraseblocks\n");
 	for (i = 0; i < ebcnt - 1; ++i) {
 		if (bbt[i] || bbt[i + 1])
 			continue;
-		set_random_data(writebuf, mtd->ecclayout->oobavail * 2);
+		prandom32_get_bytes(&rnd_state, writebuf,
+					mtd->ecclayout->oobavail * 2);
 		addr = (i + 1) * mtd->erasesize - mtd->writesize;
 		ops.mode      = MTD_OPS_AUTO_OOB;
 		ops.len       = 0;
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/9] mtd: mtd_pagetest: convert to use prandom32_get_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (4 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 6/9] mtd: mtd_oobtest: convert to use prandom32_get_bytes() Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 8/9] mtd: mtd_speedtest: use random32_get_bytes Akinobu Mita
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

This removes home-brewed pseudo-random number generator and use
prandom32_get_bytes().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/tests/mtd_pagetest.c | 43 ++++++++++++----------------------------
 1 file changed, 13 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/tests/mtd_pagetest.c b/drivers/mtd/tests/mtd_pagetest.c
index 252ddb0..ae390c3 100644
--- a/drivers/mtd/tests/mtd_pagetest.c
+++ b/drivers/mtd/tests/mtd_pagetest.c
@@ -27,6 +27,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/random.h>
 
 #define PRINT_PREF KERN_INFO "mtd_pagetest: "
 
@@ -45,26 +46,7 @@ static int bufsize;
 static int ebcnt;
 static int pgcnt;
 static int errcnt;
-static unsigned long next = 1;
-
-static inline unsigned int simple_rand(void)
-{
-	next = next * 1103515245 + 12345;
-	return (unsigned int)((next / 65536) % 32768);
-}
-
-static inline void simple_srand(unsigned long seed)
-{
-	next = seed;
-}
-
-static void set_random_data(unsigned char *buf, size_t len)
-{
-	size_t i;
-
-	for (i = 0; i < len; ++i)
-		buf[i] = simple_rand();
-}
+static struct rnd_state rnd_state;
 
 static int erase_eraseblock(int ebnum)
 {
@@ -98,7 +80,7 @@ static int write_eraseblock(int ebnum)
 	size_t written;
 	loff_t addr = ebnum * mtd->erasesize;
 
-	set_random_data(writebuf, mtd->erasesize);
+	prandom32_get_bytes(&rnd_state, writebuf, mtd->erasesize);
 	cond_resched();
 	err = mtd_write(mtd, addr, mtd->erasesize, &written, writebuf);
 	if (err || written != mtd->erasesize)
@@ -124,7 +106,7 @@ static int verify_eraseblock(int ebnum)
 	for (i = 0; i < ebcnt && bbt[ebcnt - i - 1]; ++i)
 		addrn -= mtd->erasesize;
 
-	set_random_data(writebuf, mtd->erasesize);
+	prandom32_get_bytes(&rnd_state, writebuf, mtd->erasesize);
 	for (j = 0; j < pgcnt - 1; ++j, addr += pgsize) {
 		/* Do a read to set the internal dataRAMs to different data */
 		err = mtd_read(mtd, addr0, bufsize, &read, twopages);
@@ -160,7 +142,8 @@ static int verify_eraseblock(int ebnum)
 	}
 	/* Check boundary between eraseblocks */
 	if (addr <= addrn - pgsize - pgsize && !bbt[ebnum + 1]) {
-		unsigned long oldnext = next;
+		struct rnd_state old_state = rnd_state;
+
 		/* Do a read to set the internal dataRAMs to different data */
 		err = mtd_read(mtd, addr0, bufsize, &read, twopages);
 		if (mtd_is_bitflip(err))
@@ -188,13 +171,13 @@ static int verify_eraseblock(int ebnum)
 			return err;
 		}
 		memcpy(boundary, writebuf + mtd->erasesize - pgsize, pgsize);
-		set_random_data(boundary + pgsize, pgsize);
+		prandom32_get_bytes(&rnd_state, boundary + pgsize, pgsize);
 		if (memcmp(twopages, boundary, bufsize)) {
 			printk(PRINT_PREF "error: verify failed at %#llx\n",
 			       (long long)addr);
 			errcnt += 1;
 		}
-		next = oldnext;
+		rnd_state = old_state;
 	}
 	return err;
 }
@@ -326,7 +309,7 @@ static int erasecrosstest(void)
 		return err;
 
 	printk(PRINT_PREF "writing 1st page of block %d\n", ebnum);
-	set_random_data(writebuf, pgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, pgsize);
 	strcpy(writebuf, "There is no data like this!");
 	err = mtd_write(mtd, addr0, pgsize, &written, writebuf);
 	if (err || written != pgsize) {
@@ -359,7 +342,7 @@ static int erasecrosstest(void)
 		return err;
 
 	printk(PRINT_PREF "writing 1st page of block %d\n", ebnum);
-	set_random_data(writebuf, pgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, pgsize);
 	strcpy(writebuf, "There is no data like this!");
 	err = mtd_write(mtd, addr0, pgsize, &written, writebuf);
 	if (err || written != pgsize) {
@@ -417,7 +400,7 @@ static int erasetest(void)
 		return err;
 
 	printk(PRINT_PREF "writing 1st page of block %d\n", ebnum);
-	set_random_data(writebuf, pgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, pgsize);
 	err = mtd_write(mtd, addr0, pgsize, &written, writebuf);
 	if (err || written != pgsize) {
 		printk(PRINT_PREF "error: write failed at %#llx\n",
@@ -565,7 +548,7 @@ static int __init mtd_pagetest_init(void)
 	printk(PRINT_PREF "erased %u eraseblocks\n", i);
 
 	/* Write all eraseblocks */
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	printk(PRINT_PREF "writing whole device\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
@@ -580,7 +563,7 @@ static int __init mtd_pagetest_init(void)
 	printk(PRINT_PREF "written %u eraseblocks\n", i);
 
 	/* Check all eraseblocks */
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	printk(PRINT_PREF "verifying all eraseblocks\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/9] mtd: mtd_speedtest: use random32_get_bytes
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (5 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 7/9] mtd: mtd_pagetest: " Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-28  7:19 ` [PATCH 9/9] mtd: mtd_subpagetest: convert to use prandom32_get_bytes() Akinobu Mita
  2012-10-29 20:39 ` [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Theodore Ts'o
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/tests/mtd_speedtest.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/mtd/tests/mtd_speedtest.c b/drivers/mtd/tests/mtd_speedtest.c
index 42b0f74..57f0383 100644
--- a/drivers/mtd/tests/mtd_speedtest.c
+++ b/drivers/mtd/tests/mtd_speedtest.c
@@ -49,13 +49,6 @@ static int pgcnt;
 static int goodebcnt;
 static struct timeval start, finish;
 
-static void set_random_data(unsigned char *buf, size_t len)
-{
-	size_t i;
-
-	for (i = 0; i < len; ++i)
-		buf[i] = random32();
-}
 
 static int erase_eraseblock(int ebnum)
 {
@@ -396,7 +389,7 @@ static int __init mtd_speedtest_init(void)
 		goto out;
 	}
 
-	set_random_data(iobuf, mtd->erasesize);
+	random32_get_bytes(iobuf, mtd->erasesize);
 
 	err = scan_for_bad_eraseblocks();
 	if (err)
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 9/9] mtd: mtd_subpagetest: convert to use prandom32_get_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (6 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 8/9] mtd: mtd_speedtest: use random32_get_bytes Akinobu Mita
@ 2012-10-28  7:19 ` Akinobu Mita
  2012-10-29 20:39 ` [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Theodore Ts'o
  8 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-28  7:19 UTC (permalink / raw)
  To: linux-kernel, akpm
  Cc: Akinobu Mita, Artem Bityutskiy, David Woodhouse, linux-mtd

This removes home-brewed pseudo-random number generator and use
prandom32_get_bytes().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/tests/mtd_subpagetest.c | 42 +++++++++++--------------------------
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/mtd/tests/mtd_subpagetest.c b/drivers/mtd/tests/mtd_subpagetest.c
index 9667bf5..86a7140 100644
--- a/drivers/mtd/tests/mtd_subpagetest.c
+++ b/drivers/mtd/tests/mtd_subpagetest.c
@@ -26,6 +26,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/random.h>
 
 #define PRINT_PREF KERN_INFO "mtd_subpagetest: "
 
@@ -43,26 +44,7 @@ static int bufsize;
 static int ebcnt;
 static int pgcnt;
 static int errcnt;
-static unsigned long next = 1;
-
-static inline unsigned int simple_rand(void)
-{
-	next = next * 1103515245 + 12345;
-	return (unsigned int)((next / 65536) % 32768);
-}
-
-static inline void simple_srand(unsigned long seed)
-{
-	next = seed;
-}
-
-static void set_random_data(unsigned char *buf, size_t len)
-{
-	size_t i;
-
-	for (i = 0; i < len; ++i)
-		buf[i] = simple_rand();
-}
+static struct rnd_state rnd_state;
 
 static inline void clear_data(unsigned char *buf, size_t len)
 {
@@ -119,7 +101,7 @@ static int write_eraseblock(int ebnum)
 	int err = 0;
 	loff_t addr = ebnum * mtd->erasesize;
 
-	set_random_data(writebuf, subpgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, subpgsize);
 	err = mtd_write(mtd, addr, subpgsize, &written, writebuf);
 	if (unlikely(err || written != subpgsize)) {
 		printk(PRINT_PREF "error: write failed at %#llx\n",
@@ -133,7 +115,7 @@ static int write_eraseblock(int ebnum)
 
 	addr += subpgsize;
 
-	set_random_data(writebuf, subpgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, subpgsize);
 	err = mtd_write(mtd, addr, subpgsize, &written, writebuf);
 	if (unlikely(err || written != subpgsize)) {
 		printk(PRINT_PREF "error: write failed at %#llx\n",
@@ -157,7 +139,7 @@ static int write_eraseblock2(int ebnum)
 	for (k = 1; k < 33; ++k) {
 		if (addr + (subpgsize * k) > (ebnum + 1) * mtd->erasesize)
 			break;
-		set_random_data(writebuf, subpgsize * k);
+		prandom32_get_bytes(&rnd_state, writebuf, subpgsize * k);
 		err = mtd_write(mtd, addr, subpgsize * k, &written, writebuf);
 		if (unlikely(err || written != subpgsize * k)) {
 			printk(PRINT_PREF "error: write failed at %#llx\n",
@@ -193,7 +175,7 @@ static int verify_eraseblock(int ebnum)
 	int err = 0;
 	loff_t addr = ebnum * mtd->erasesize;
 
-	set_random_data(writebuf, subpgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, subpgsize);
 	clear_data(readbuf, subpgsize);
 	err = mtd_read(mtd, addr, subpgsize, &read, readbuf);
 	if (unlikely(err || read != subpgsize)) {
@@ -220,7 +202,7 @@ static int verify_eraseblock(int ebnum)
 
 	addr += subpgsize;
 
-	set_random_data(writebuf, subpgsize);
+	prandom32_get_bytes(&rnd_state, writebuf, subpgsize);
 	clear_data(readbuf, subpgsize);
 	err = mtd_read(mtd, addr, subpgsize, &read, readbuf);
 	if (unlikely(err || read != subpgsize)) {
@@ -257,7 +239,7 @@ static int verify_eraseblock2(int ebnum)
 	for (k = 1; k < 33; ++k) {
 		if (addr + (subpgsize * k) > (ebnum + 1) * mtd->erasesize)
 			break;
-		set_random_data(writebuf, subpgsize * k);
+		prandom32_get_bytes(&rnd_state, writebuf, subpgsize * k);
 		clear_data(readbuf, subpgsize * k);
 		err = mtd_read(mtd, addr, subpgsize * k, &read, readbuf);
 		if (unlikely(err || read != subpgsize * k)) {
@@ -430,7 +412,7 @@ static int __init mtd_subpagetest_init(void)
 		goto out;
 
 	printk(PRINT_PREF "writing whole device\n");
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
 			continue;
@@ -443,7 +425,7 @@ static int __init mtd_subpagetest_init(void)
 	}
 	printk(PRINT_PREF "written %u eraseblocks\n", i);
 
-	simple_srand(1);
+	prandom32_seed(&rnd_state, 1);
 	printk(PRINT_PREF "verifying all eraseblocks\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
@@ -466,7 +448,7 @@ static int __init mtd_subpagetest_init(void)
 		goto out;
 
 	/* Write all eraseblocks */
-	simple_srand(3);
+	prandom32_seed(&rnd_state, 3);
 	printk(PRINT_PREF "writing whole device\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
@@ -481,7 +463,7 @@ static int __init mtd_subpagetest_init(void)
 	printk(PRINT_PREF "written %u eraseblocks\n", i);
 
 	/* Check all eraseblocks */
-	simple_srand(3);
+	prandom32_seed(&rnd_state, 3);
 	printk(PRINT_PREF "verifying all eraseblocks\n");
 	for (i = 0; i < ebcnt; ++i) {
 		if (bbt[i])
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
  2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
                   ` (7 preceding siblings ...)
  2012-10-28  7:19 ` [PATCH 9/9] mtd: mtd_subpagetest: convert to use prandom32_get_bytes() Akinobu Mita
@ 2012-10-29 20:39 ` Theodore Ts'o
  2012-10-30 11:01   ` Akinobu Mita
  8 siblings, 1 reply; 20+ messages in thread
From: Theodore Ts'o @ 2012-10-29 20:39 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-kernel, akpm, Artem Bityutskiy, Adrian Hunter,
	David Woodhouse, linux-mtd

On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote:
>  /**
> + *	prandom32_get_bytes - get the requested number of pseudo-random bytes
> + *	@state: pointer to state structure holding seeded state.
> + *	@buf: where to copy the pseudo-random bytes to
> + *	@bytes: the requested number of bytes
> + *
> + *	This is used for pseudo-randomness with no outside seeding.
> + *	For more random results, use random32_get_bytes().
> + */
> +
> +/**
> + *	random32_get_bytes - get the requested number of pseudo-random bytes
> + *	@buf: where to copy the pseudo-random bytes to
> + *	@bytes: the requested number of bytes
> + */

This naming scheme is going to be very confusing.  If the function is
going to return a pseudo-random number, it *must* have a "prandom"
suffix.  Otherwise some kernel developer, somewhere, will get confused
between get_random_bytes() and random32_get_bytes(), and the result
may be a very embarassing security exposure.

How about prandom32_get_bytes_state() and prandom32_get_bytes() instead?

						- Ted

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-28  7:18 ` [PATCH 2/9] uuid: use random32_get_bytes() Akinobu Mita
@ 2012-10-29 20:52   ` Theodore Ts'o
  2012-10-30  1:49     ` Huang Ying
  0 siblings, 1 reply; 20+ messages in thread
From: Theodore Ts'o @ 2012-10-29 20:52 UTC (permalink / raw)
  To: Akinobu Mita; +Cc: linux-kernel, akpm, ying.huang, chris.mason, linux-btrfs

On Sun, Oct 28, 2012 at 04:18:59PM +0900, Akinobu Mita wrote:
> Use random32_get_bytes() to generate 16 bytes of pseudo-random bytes.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

Since your patch is going to allow users to set the random seed, it
means that what had previously been a bad security bug has just become
a grievous security bug.  If you are going to be generating UUID's
they _must_ use a truly random random generator, since the whole point
of uuid's is that they be unique.  If someone can trivially set the
random seed of a prng, and thus cause the uuid generator to generate,
well, non-unique UUID's, the results can range anywhere from
confusion, to file system corruption and data loss.

Fortunately, there is only one user of lib/uuid.c, and that's the
btrfs file system.

Chris and the Btrfs folks --- my recommendation would be to ditch the
use of uuid_be_gen, "git rm lib/uuid.c" with extreme prejudice, and
use generate_random_uuid() which was coded over a decade ago in
drivers/char/random.c.  Not only does this properly use the kernel
random number generator, but it also creates a UUID with the correct
format.  (It's not enough to set the UUID version to 4; you also need
to set the UUID variant to be DCE if you want to be properly compliant
with RFC 4122 --- see section 4.1.1.)

The btrfs file system doesn't generate uuid's in any critical fast
paths as near as I can determine, so it should be perfectly safe to
use uuid_generate() --- I also would drop the whole distinction
between little-endian and big-endian uuid's, BTW.  RFC 4122 is quite
explicit about how uuid's should be encoded, and it's in internet byte
order.  This is what OSF/DCE uses, and it's what the rest of the world
(Microsoft, SAP AG, Apple, GNOME, KDE) uses as well.

Regards,

					- Ted

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-29 20:52   ` Theodore Ts'o
@ 2012-10-30  1:49     ` Huang Ying
  2012-10-30  4:48       ` Theodore Ts'o
  0 siblings, 1 reply; 20+ messages in thread
From: Huang Ying @ 2012-10-30  1:49 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Akinobu Mita, linux-kernel, akpm, chris.mason, linux-btrfs

On Mon, 2012-10-29 at 16:52 -0400, Theodore Ts'o wrote:
> On Sun, Oct 28, 2012 at 04:18:59PM +0900, Akinobu Mita wrote:
> > Use random32_get_bytes() to generate 16 bytes of pseudo-random bytes.
> > 
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> 
> Since your patch is going to allow users to set the random seed, it
> means that what had previously been a bad security bug has just become
> a grievous security bug.  If you are going to be generating UUID's
> they _must_ use a truly random random generator, since the whole point
> of uuid's is that they be unique.  If someone can trivially set the
> random seed of a prng, and thus cause the uuid generator to generate,
> well, non-unique UUID's, the results can range anywhere from
> confusion, to file system corruption and data loss.
> 
> Fortunately, there is only one user of lib/uuid.c, and that's the
> btrfs file system.
> 
> Chris and the Btrfs folks --- my recommendation would be to ditch the
> use of uuid_be_gen, "git rm lib/uuid.c" with extreme prejudice, and
> use generate_random_uuid() which was coded over a decade ago in
> drivers/char/random.c.  Not only does this properly use the kernel
> random number generator, but it also creates a UUID with the correct
> format.  (It's not enough to set the UUID version to 4; you also need
> to set the UUID variant to be DCE if you want to be properly compliant
> with RFC 4122 --- see section 4.1.1.)

The uuid_le/be_gen() in lib/uuid.c has set UUID variants to be DCE,
that is done in __uuid_gen_common() with "b[8] = (b[8] & 0x3F) | 0x80".

To deal with random number generation issue, how about use
get_random_bytes() in __uuid_gen_common()?

> The btrfs file system doesn't generate uuid's in any critical fast
> paths as near as I can determine, so it should be perfectly safe to
> use uuid_generate() --- I also would drop the whole distinction
> between little-endian and big-endian uuid's, BTW.  RFC 4122 is quite
> explicit about how uuid's should be encoded, and it's in internet byte
> order.  This is what OSF/DCE uses, and it's what the rest of the world
> (Microsoft, SAP AG, Apple, GNOME, KDE) uses as well.

uuid_be complies RFC4122, it uses internet byte order.  But EFI uses
little endian.

Best Regards,
Huang Ying



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-30  1:49     ` Huang Ying
@ 2012-10-30  4:48       ` Theodore Ts'o
  2012-10-31  1:35         ` Huang Ying
  0 siblings, 1 reply; 20+ messages in thread
From: Theodore Ts'o @ 2012-10-30  4:48 UTC (permalink / raw)
  To: Huang Ying; +Cc: Akinobu Mita, linux-kernel, akpm, chris.mason, linux-btrfs

On Tue, Oct 30, 2012 at 09:49:58AM +0800, Huang Ying wrote:
> The uuid_le/be_gen() in lib/uuid.c has set UUID variants to be DCE,
> that is done in __uuid_gen_common() with "b[8] = (b[8] & 0x3F) | 0x80".

Oh, I see, I missed that.

> To deal with random number generation issue, how about use
> get_random_bytes() in __uuid_gen_common()?

We already have generate_random_uuid() in drivers/char/random.c, and
no users for lib/uuid.c's equivalent uuid_be_gen().  So here's a
counter-proposal, why don't we drop lib/uuid.c, and include in
drivers/char/random.c:

/*
 * Generate random GUID
 *
 * GUID's is like UUID's, but they uses the non-standard little-endian
 * layout, compared to what is defined in RFC-4112; it is primarily
 * used by the EFI specification.
 */
void generate_random_guid(unsigned char uuid_out[16])
{
	get_random_bytes(uuid_out, 16);
	/* Set UUID version to 4 --- truly random generation */
	uuid_out[7] = (uuid_out[7] & 0x0F) | 0x40;
	/* Set the UUID variant to DCE */
	uuid_out[8] = (uuid_out[8] & 0x3F) | 0x80;
}
EXPORT_SYMBOL(generate_random_guid);

I really don't think it's worth it to have a __uuid_gen_common once we
are using get_random_bytes(), since there isn't much code to be
factored out, and it's simpler just to have two functions in one place.

Using UUID vs. GUID I think makes things much clearer, since the EFI
specification talks about GUID's, not UUID's, and that way we don't
have to worry about people getting confused about whether they should
be using the little-endian versus big-endian variant.  (And I'd love
to ask to whoever wrote the EFI specification what on *Earth* were
they thinking when they decided to diverge from the rest of the
world....)

	      	       	       	       	    - Ted

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
  2012-10-29 20:39 ` [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Theodore Ts'o
@ 2012-10-30 11:01   ` Akinobu Mita
  2012-10-30 11:12     ` Akinobu Mita
  0 siblings, 1 reply; 20+ messages in thread
From: Akinobu Mita @ 2012-10-30 11:01 UTC (permalink / raw)
  To: Theodore Ts'o, Akinobu Mita, linux-kernel, akpm,
	Artem Bityutskiy, Adrian Hunter, David Woodhouse, linux-mtd

2012/10/30 Theodore Ts'o <tytso@mit.edu>:
> On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote:
>>  /**
>> + *   prandom32_get_bytes - get the requested number of pseudo-random bytes
>> + *   @state: pointer to state structure holding seeded state.
>> + *   @buf: where to copy the pseudo-random bytes to
>> + *   @bytes: the requested number of bytes
>> + *
>> + *   This is used for pseudo-randomness with no outside seeding.
>> + *   For more random results, use random32_get_bytes().
>> + */
>> +
>> +/**
>> + *   random32_get_bytes - get the requested number of pseudo-random bytes
>> + *   @buf: where to copy the pseudo-random bytes to
>> + *   @bytes: the requested number of bytes
>> + */
>
> This naming scheme is going to be very confusing.  If the function is
> going to return a pseudo-random number, it *must* have a "prandom"
> suffix.  Otherwise some kernel developer, somewhere, will get confused
> between get_random_bytes() and random32_get_bytes(), and the result
> may be a very embarassing security exposure.
>
> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead?

I agree with your suggestion.  I'll rename them and try again.

By the way, should we also rename the existing random32() and
prandom32() in the future?

Specifically, rename random32() to prandom32(), and prandom32() to
prandom32_state().  As a result, it will cause a little confusion
between old and new prandom32(). But the number of arguments will
be changed from 3 to 2, so gcc can detect the misuse of prandom32().

Is there any other idea of renaming? Or should we keep them as is?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
  2012-10-30 11:01   ` Akinobu Mita
@ 2012-10-30 11:12     ` Akinobu Mita
  2012-10-31  3:29       ` Theodore Ts'o
  0 siblings, 1 reply; 20+ messages in thread
From: Akinobu Mita @ 2012-10-30 11:12 UTC (permalink / raw)
  To: Theodore Ts'o, Akinobu Mita, linux-kernel, akpm,
	Artem Bityutskiy, Adrian Hunter, David Woodhouse, linux-mtd

2012/10/30 Akinobu Mita <akinobu.mita@gmail.com>:
> 2012/10/30 Theodore Ts'o <tytso@mit.edu>:
>> On Sun, Oct 28, 2012 at 04:18:58PM +0900, Akinobu Mita wrote:
>>>  /**
>>> + *   prandom32_get_bytes - get the requested number of pseudo-random bytes
>>> + *   @state: pointer to state structure holding seeded state.
>>> + *   @buf: where to copy the pseudo-random bytes to
>>> + *   @bytes: the requested number of bytes
>>> + *
>>> + *   This is used for pseudo-randomness with no outside seeding.
>>> + *   For more random results, use random32_get_bytes().
>>> + */
>>> +
>>> +/**
>>> + *   random32_get_bytes - get the requested number of pseudo-random bytes
>>> + *   @buf: where to copy the pseudo-random bytes to
>>> + *   @bytes: the requested number of bytes
>>> + */
>>
>> This naming scheme is going to be very confusing.  If the function is
>> going to return a pseudo-random number, it *must* have a "prandom"
>> suffix.  Otherwise some kernel developer, somewhere, will get confused
>> between get_random_bytes() and random32_get_bytes(), and the result
>> may be a very embarassing security exposure.
>>
>> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead?
>
> I agree with your suggestion.  I'll rename them and try again.
>
> By the way, should we also rename the existing random32() and
> prandom32() in the future?
>
> Specifically, rename random32() to prandom32(), and prandom32() to
> prandom32_state().  As a result, it will cause a little confusion
> between old and new prandom32(). But the number of arguments will
> be changed from 3 to 2, so gcc can detect the misuse of prandom32().

Oops, I intended to say "the number of arguments of prandom32() will be
changed from 1 to 0". And I realized that the exisiting srandom32() also should
be renamed to sprandom32().

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-30  4:48       ` Theodore Ts'o
@ 2012-10-31  1:35         ` Huang Ying
  2012-10-31  2:38           ` Theodore Ts'o
  0 siblings, 1 reply; 20+ messages in thread
From: Huang Ying @ 2012-10-31  1:35 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Akinobu Mita, linux-kernel, akpm, chris.mason, linux-btrfs

On Tue, 2012-10-30 at 00:48 -0400, Theodore Ts'o wrote:
> On Tue, Oct 30, 2012 at 09:49:58AM +0800, Huang Ying wrote:
> > The uuid_le/be_gen() in lib/uuid.c has set UUID variants to be DCE,
> > that is done in __uuid_gen_common() with "b[8] = (b[8] & 0x3F) | 0x80".
> 
> Oh, I see, I missed that.
> 
> > To deal with random number generation issue, how about use
> > get_random_bytes() in __uuid_gen_common()?
> 
> We already have generate_random_uuid() in drivers/char/random.c, and
> no users for lib/uuid.c's equivalent uuid_be_gen().  So here's a
> counter-proposal, why don't we drop lib/uuid.c, and include in
> drivers/char/random.c:
> 
> /*
>  * Generate random GUID
>  *
>  * GUID's is like UUID's, but they uses the non-standard little-endian
>  * layout, compared to what is defined in RFC-4112; it is primarily
>  * used by the EFI specification.
>  */
> void generate_random_guid(unsigned char uuid_out[16])
> {
> 	get_random_bytes(uuid_out, 16);
> 	/* Set UUID version to 4 --- truly random generation */
> 	uuid_out[7] = (uuid_out[7] & 0x0F) | 0x40;
> 	/* Set the UUID variant to DCE */
> 	uuid_out[8] = (uuid_out[8] & 0x3F) | 0x80;
> }
> EXPORT_SYMBOL(generate_random_guid);
> 
> I really don't think it's worth it to have a __uuid_gen_common once we
> are using get_random_bytes(), since there isn't much code to be
> factored out, and it's simpler just to have two functions in one place.

The intention of lib/uuid.c is to unify various UUID related code, and
put them in same place.  In addition to UUID generation, it provide some
other utility and may provide/collect more in the future.  So do you
think it is a good idea to put generate_rand_uuid/guid into lib/uuid.c
and maybe change the name/prototype to make it consistent with other
uuid definitions?

> Using UUID vs. GUID I think makes things much clearer, since the EFI
> specification talks about GUID's, not UUID's, and that way we don't
> have to worry about people getting confused about whether they should
> be using the little-endian versus big-endian variant.  (And I'd love
> to ask to whoever wrote the EFI specification what on *Earth* were
> they thinking when they decided to diverge from the rest of the
> world....)

I think that is a good idea.  From Wikipedia, GUID is in native byte
order, while UUID is in internet byte order.

Best Regards,
Huang Ying



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-31  1:35         ` Huang Ying
@ 2012-10-31  2:38           ` Theodore Ts'o
  2012-10-31  3:06             ` Huang Ying
  0 siblings, 1 reply; 20+ messages in thread
From: Theodore Ts'o @ 2012-10-31  2:38 UTC (permalink / raw)
  To: Huang Ying; +Cc: Akinobu Mita, linux-kernel, akpm, chris.mason, linux-btrfs

On Wed, Oct 31, 2012 at 09:35:37AM +0800, Huang Ying wrote:
> 
> The intention of lib/uuid.c is to unify various UUID related code, and
> put them in same place.  In addition to UUID generation, it provide some
> other utility and may provide/collect more in the future.  So do you
> think it is a good idea to put generate_rand_uuid/guid into lib/uuid.c
> and maybe change the name/prototype to make it consistent with other
> uuid definitions?

I had trouble understanding why lib/uuid.c existed, since the only
thing I saw was the uuid generation function.  After some more
looking, I see you also created inline functions which wrapped
memcmp().

The problem I have with your abstractions is that it just makes life
more complicated for the callers.  All of the current places which use
generate_random_uuid() merely want to fill in a unsigned char array.
This includes btrfs, by the way, which is already using
generate_random_uuid in some places, and I'm not sure why they are
using uuid_le_gen(), since there doesn't seem to be any need for a
little-endian uuid/guid here (it's just used as unique bag of bits
which is 16 bytes long), and using uuid_le_gen() means extra memory
has to be allocated on the stack, and then an extra memory copy is
required.  Contrast (in fs/btrfs/root-tree.c):

	   uuid_le uuid;
	   ...
		uuid_le_gen(&uuid);
		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);

versus, simply doing (fs/btrfs/volumes.c):

	generate_random_uuid(fs_devices->fsid);

see which one is easier?  And after the uuid is generated, none of the
current callers ever do any manipulation of the uuid, so there's no
real point to play fancy typedef games; it just adds more work for no
real gain.

> > Using UUID vs. GUID I think makes things much clearer, since the EFI
> > specification talks about GUID's, not UUID's, and that way we don't
> > have to worry about people getting confused about whether they should
> > be using the little-endian versus big-endian variant.  (And I'd love
> > to ask to whoever wrote the EFI specification what on *Earth* were
> > they thinking when they decided to diverge from the rest of the
> > world....)
> 
> I think that is a good idea.  From Wikipedia, GUID is in native byte
> order, while UUID is in internet byte order.

Well, technially GUID is "intel/little-endian byte order".  If someone
tried to implement the GPT on a big-endian system, such as PowerPC,
they would still have to use the little-endian byte order, even though
it's not the native byte order for that architecture.  Otherwise
devices wouldn't be portable between those systems.  (This is why I
think the GUID was such a bad idea; everyone basically treats them as
16 byte octet strings, so this whole idea of "native byte order" just
to save a few byte swaps at UUID generation time was really, IMHO, a
very bad idea.)

Regards,

					- Ted

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/9] uuid: use random32_get_bytes()
  2012-10-31  2:38           ` Theodore Ts'o
@ 2012-10-31  3:06             ` Huang Ying
  0 siblings, 0 replies; 20+ messages in thread
From: Huang Ying @ 2012-10-31  3:06 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Akinobu Mita, linux-kernel, akpm, chris.mason, linux-btrfs

On Tue, 2012-10-30 at 22:38 -0400, Theodore Ts'o wrote:
> On Wed, Oct 31, 2012 at 09:35:37AM +0800, Huang Ying wrote:
> > 
> > The intention of lib/uuid.c is to unify various UUID related code, and
> > put them in same place.  In addition to UUID generation, it provide some
> > other utility and may provide/collect more in the future.  So do you
> > think it is a good idea to put generate_rand_uuid/guid into lib/uuid.c
> > and maybe change the name/prototype to make it consistent with other
> > uuid definitions?
> 
> I had trouble understanding why lib/uuid.c existed, since the only
> thing I saw was the uuid generation function.  After some more
> looking, I see you also created inline functions which wrapped
> memcmp().
> 
> The problem I have with your abstractions is that it just makes life
> more complicated for the callers.  All of the current places which use
> generate_random_uuid() merely want to fill in a unsigned char array.
> This includes btrfs, by the way, which is already using
> generate_random_uuid in some places, and I'm not sure why they are
> using uuid_le_gen(), since there doesn't seem to be any need for a
> little-endian uuid/guid here (it's just used as unique bag of bits
> which is 16 bytes long), and using uuid_le_gen() means extra memory
> has to be allocated on the stack, and then an extra memory copy is
> required.  Contrast (in fs/btrfs/root-tree.c):
> 
> 	   uuid_le uuid;
> 	   ...
> 		uuid_le_gen(&uuid);
> 		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
> 
> versus, simply doing (fs/btrfs/volumes.c):
> 
> 	generate_random_uuid(fs_devices->fsid);
> 
> see which one is easier?  And after the uuid is generated, none of the
> current callers ever do any manipulation of the uuid, so there's no
> real point to play fancy typedef games; it just adds more work for no
> real gain.

If we use uuid_le when we define the data structure, life will be eaiser

struct btrfs_root_item {
	...
	uuid_le uuid;
	...
};

Then it is quite easy to use it.

uuid_le_gen(&item->uuid);

That is the intended usage model.

UUID_LE() macro definition has some user.  It makes it easier to
construct UUID/GUID defined in some specs.

> > > Using UUID vs. GUID I think makes things much clearer, since the EFI
> > > specification talks about GUID's, not UUID's, and that way we don't
> > > have to worry about people getting confused about whether they should
> > > be using the little-endian versus big-endian variant.  (And I'd love
> > > to ask to whoever wrote the EFI specification what on *Earth* were
> > > they thinking when they decided to diverge from the rest of the
> > > world....)
> > 
> > I think that is a good idea.  From Wikipedia, GUID is in native byte
> > order, while UUID is in internet byte order.
> 
> Well, technially GUID is "intel/little-endian byte order".  If someone
> tried to implement the GPT on a big-endian system, such as PowerPC,
> they would still have to use the little-endian byte order, even though
> it's not the native byte order for that architecture.  Otherwise
> devices wouldn't be portable between those systems.  (This is why I
> think the GUID was such a bad idea; everyone basically treats them as
> 16 byte octet strings, so this whole idea of "native byte order" just
> to save a few byte swaps at UUID generation time was really, IMHO, a
> very bad idea.)

Yes.  Explicit byte order is better.

Best Regards,
Huang Ying



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
  2012-10-30 11:12     ` Akinobu Mita
@ 2012-10-31  3:29       ` Theodore Ts'o
  2012-10-31 11:53         ` Akinobu Mita
  0 siblings, 1 reply; 20+ messages in thread
From: Theodore Ts'o @ 2012-10-31  3:29 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-kernel, akpm, Artem Bityutskiy, Adrian Hunter,
	David Woodhouse, linux-mtd

On Tue, Oct 30, 2012 at 08:12:39PM +0900, Akinobu Mita wrote:
> >>
> >> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead?
> >
> > I agree with your suggestion.  I'll rename them and try again.
> >
> > By the way, should we also rename the existing random32() and
> > prandom32() in the future?

I suppose the other way to go is to just use random32 as the common
prefix, and just have random32() and random32_state().  My concern was
that people might assume that prandom32() and random32() might imply
that only prandom32() was the one using a pseudo-random number
generator.  This might be easier since there are large number of uses
of random32() in the source tree, but only a relative few using
prandom32().

    	   	     	    	     - Ted

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes()
  2012-10-31  3:29       ` Theodore Ts'o
@ 2012-10-31 11:53         ` Akinobu Mita
  0 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2012-10-31 11:53 UTC (permalink / raw)
  To: Theodore Ts'o, Akinobu Mita, linux-kernel, akpm,
	Artem Bityutskiy, Adrian Hunter, David Woodhouse, linux-mtd

2012/10/31 Theodore Ts'o <tytso@mit.edu>:
> On Tue, Oct 30, 2012 at 08:12:39PM +0900, Akinobu Mita wrote:
>> >>
>> >> How about prandom32_get_bytes_state() and prandom32_get_bytes() instead?
>> >
>> > I agree with your suggestion.  I'll rename them and try again.
>> >
>> > By the way, should we also rename the existing random32() and
>> > prandom32() in the future?
>
> I suppose the other way to go is to just use random32 as the common
> prefix, and just have random32() and random32_state().  My concern was
> that people might assume that prandom32() and random32() might imply
> that only prandom32() was the one using a pseudo-random number
> generator.  This might be easier since there are large number of uses
> of random32() in the source tree, but only a relative few using
> prandom32().

Using random32 as the common prefix sounds good idea.  I'm going to
prepare the following patch set:

[patch 1] rename prandom32 to randome32_state
[patch 2] introduce random32_get_bytes and random32_get_bytes_state
[patch 3-] proliferate random32_get_bytes and random32_get_bytes_state

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-10-31 11:53 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-28  7:18 [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Akinobu Mita
2012-10-28  7:18 ` [PATCH 2/9] uuid: use random32_get_bytes() Akinobu Mita
2012-10-29 20:52   ` Theodore Ts'o
2012-10-30  1:49     ` Huang Ying
2012-10-30  4:48       ` Theodore Ts'o
2012-10-31  1:35         ` Huang Ying
2012-10-31  2:38           ` Theodore Ts'o
2012-10-31  3:06             ` Huang Ying
2012-10-28  7:19 ` [PATCH 3/9] mtd: mtd_nandecctest: use random32_get_bytes instead of get_random_bytes() Akinobu Mita
2012-10-28  7:19 ` [PATCH 4/9] mtd: nandsim: use random32_get_bytes Akinobu Mita
2012-10-28  7:19 ` [PATCH 5/9] ubifs: " Akinobu Mita
2012-10-28  7:19 ` [PATCH 6/9] mtd: mtd_oobtest: convert to use prandom32_get_bytes() Akinobu Mita
2012-10-28  7:19 ` [PATCH 7/9] mtd: mtd_pagetest: " Akinobu Mita
2012-10-28  7:19 ` [PATCH 8/9] mtd: mtd_speedtest: use random32_get_bytes Akinobu Mita
2012-10-28  7:19 ` [PATCH 9/9] mtd: mtd_subpagetest: convert to use prandom32_get_bytes() Akinobu Mita
2012-10-29 20:39 ` [PATCH 1/9] random32: introduce random32_get_bytes() and prandom32_get_bytes() Theodore Ts'o
2012-10-30 11:01   ` Akinobu Mita
2012-10-30 11:12     ` Akinobu Mita
2012-10-31  3:29       ` Theodore Ts'o
2012-10-31 11:53         ` Akinobu Mita

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).