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
| ||
|
Date: Mon, 18 Apr 2022 20:53:01 +0200 From: "Jason A. Donenfeld" <Jason@...c4.com> To: Eric Biggers <ebiggers@...nel.org> Cc: linux-kernel@...r.kernel.org, Dominik Brodowski <linux@...inikbrodowski.net> Subject: Re: [PATCH] random: use memmove instead of memcpy for remaining 32 bytes Hey Eric, On Mon, Apr 18, 2022 at 11:43:11AM -0700, Eric Biggers wrote: > On Thu, Apr 14, 2022 at 01:56:49AM +0200, Jason A. Donenfeld wrote: > > In order to immediately overwrite the old key on the stack, before > > servicing a userspace request for bytes, we use the remaining 32 bytes > > of block 0 as the key. This means moving indices 8,9,a,b,c,d,e,f -> > > 4,5,6,7,8,9,a,b. Since 4 < 8, for the kernel implementations of > > memcpy(), this doesn't actually appear to be a problem in practice. But > > relying on that characteristic seems a bit brittle. So let's change that > > to a proper memmove(), which is the by-the-books way of handling > > overlapping memory copies. > > > > Cc: Dominik Brodowski <linux@...inikbrodowski.net> > > Signed-off-by: Jason A. Donenfeld <Jason@...c4.com> > > --- > > drivers/char/random.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/char/random.c b/drivers/char/random.c > > index 6b01b2be9dd4..3a293f919af9 100644 > > --- a/drivers/char/random.c > > +++ b/drivers/char/random.c > > @@ -333,7 +333,7 @@ static void crng_fast_key_erasure(u8 key[CHACHA_KEY_SIZE], > > chacha20_block(chacha_state, first_block); > > > > memcpy(key, first_block, CHACHA_KEY_SIZE); > > - memcpy(random_data, first_block + CHACHA_KEY_SIZE, random_data_len); > > + memmove(random_data, first_block + CHACHA_KEY_SIZE, random_data_len); > > memzero_explicit(first_block, sizeof(first_block)); > > } > > first_block is on the stack, so this is never an overlapping copy. Oh, grrr, yes you're right. I clearly over thought this into non-sense. Will revert. > > It would be more important to document the fact that random_data can point into > chacha_state, as this is not obvious. Good idea. I'll do that. Jason
Powered by blists - more mailing lists