On Sat, 20 Oct 2018, Conrad Meyer wrote:
... Log: Fortuna: trivial static variable cleanupRemove unnecessary use of function-local static variable. 32 bytes is small enough to live on the stack. ... Modified: head/sys/dev/random/fortuna.c ============================================================================== --- head/sys/dev/random/fortuna.c Sat Oct 20 20:12:57 2018 (r339480) +++ head/sys/dev/random/fortuna.c Sat Oct 20 20:15:06 2018 (r339481) @@ -333,7 +333,7 @@ random_fortuna_genblocks(uint8_t *buf, u_int blockcoun static __inline void random_fortuna_genrandom(uint8_t *buf, u_int bytecount) { - static uint8_t temp[RANDOM_BLOCKSIZE*(RANDOM_KEYS_PER_BLOCK)]; + uint8_t temp[RANDOM_BLOCKSIZE * RANDOM_KEYS_PER_BLOCK]; u_int blockcount; RANDOM_RESEED_ASSERT_LOCK_OWNED();
This now uses explicit_bzero() on the stack variable (as before, just before returning from the function). This is neither necessary nor sufficient for security. Even with the static variable, it was a reasonable optimization to keep a copy of the whole variable on the stack, or better in registers which might be spilled to the stack (this is possible because the variable is small). So to be really secure, the function must clear all stack and registers used by the function. In practice, something probably clobbers the stack and registers before any untrusted thread can read them, and the explicit_bzero() is not really needed. However, this function is explicitly bogusly inlined, so the stack and registers usually don't get clobbered immediately after the function returns. The inlining is bogus since this function is very large. The function call overhead for just the explicit_bzero() in it is as large as the possible savings from inlining it. clang likes to bogusly inline almost all static functions. If this one were not too large to inline, then clang would inline it even if it is not declared __inline, so declaring it has no good effect if the compiler is clang. Auto-inlining may open security holes by extending the lifetime of stack variables. That is another reason to not do it. I use gcc -fno-inline -fno-inline-functions-called-once to kill all auto-inlining. clang inlines even functions called more than once, and doesn't support these flags. Where was the security hole with the old static variable? I think it was mostly with the long lifetime of the variable. The global memory is actually more secure against accidental leaks by copying it out than stack memory. But even local variables have long lifetimes if you can manage to suspend the system while a critical local variable is active. I think rendezvous stuff prevents this particalar variable remaining active across suspend. Bruce _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
