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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 04 Aug 2014 11:48:33 +0200 From: Daniel Borkmann <dborkman@...hat.com> To: David Miller <davem@...emloft.net> CC: netdev@...r.kernel.org, hannes@...essinduktion.org, akinobu.mita@...il.com Subject: Re: [PATCH net-next 1/2] random32: improvements to prandom_bytes [ Cc'ing Akinobu, 6582c665d6b882 ("prandom: introduce prandom_bytes() and prandom_bytes_state()") ] On 08/01/2014 07:28 AM, David Miller wrote: > From: Daniel Borkmann <dborkman@...hat.com> > Date: Thu, 31 Jul 2014 22:11:16 +0200 > >> - for (j = 0; j < sizeof(u32); j++) { >> - p[i + j] = random; >> - random >>= BITS_PER_BYTE; >> - } >> + while (bytes > sizeof(u32)) { [ Should have been >= here, missed that earlier. ] >> + put_unaligned(prandom_u32_state(state), (u32 *) ptr); >> + ptr += sizeof(u32); >> + bytes -= sizeof(u32); >> } >> - if (i < bytes) { >> - u32 random = prandom_u32_state(state); >> >> - for (; i < bytes; i++) { >> - p[i] = random; >> - random >>= BITS_PER_BYTE; >> - } >> + if (bytes > 0) { >> + u32 rem = prandom_u32_state(state); >> + do { >> + *ptr++ = (u8) rem; >> + bytes--; >> + rem >>= BITS_PER_BYTE; >> + } while (bytes > 0); >> } > > This conversion to put_unaligned() is not an equivalent depending > upon the endianness of the cpu. > > And this means the random value gets distributed differently into > full words than it will into trailing bytes. > > Let's just not mess around with this, ok? Sorry for the late answer, Dave. So for the case of callers of prandom_bytes() which internally invoke prandom_bytes_state(), it doesn't matter at all since, well, they expect the array to be filled randomly and don't have any control of the internal state anyway (that's also why we have periodic reseeding there, etc), so they really don't care. Now for the direct callers of prandom_bytes_state(), which are solely located in drivers/mtd/tests/{oobtest.c, pagetest.c, subpagetest.c}: These tests basically fill a test write-vector through prandom_bytes_state() with an a-priori defined seed each time and write that to a MTD device. Later on, they set up a read-vector and read back that blocks from the device. So in the verification phase, the write-vector is being re-setup [ so same seed and prandom_bytes_state() called ], and then memcmp()'ed against the read-vector to check if the data is the same. Akinobu and I also tested this patch with the above fix included, and it runs through the 3 relevant test cases _without_ any errors on the nandsim device (simulator for MTD devs): # modprobe nandsim first_id_byte=0x20 second_id_byte=0xac \ third_id_byte=0x00 fourth_id_byte=0x15 # modprobe mtd_oobtest dev=0 # modprobe mtd_pagetest dev=0 # modprobe mtd_subpagetest dev=0 [ ... 'finished with 0 errors' each in klog ] We also don't have any users depending directly on a particular result of the PRNG (except the PRNG self-test itself), and that's just fine as it e.g. allowed us easily to do things like upgrading from taus88 to taus113. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists