From 5528565a76f5caae336d4f13213108dc1fad4ae0 Mon Sep 17 00:00:00 2001 From: Conrad Meyer Date: Sat, 20 Oct 2018 21:09:12 +0000 Subject: [PATCH] Fortuna: Fix a race to prevent reseed spamming If multiple threads enter fortuna_pre_read contemporaneously, such as via read(2) or getrandom(2), they could race to check how long it has been since the last update due to a TOCTOU problem with 'now'. Here is an example problematic execution: Thread A: Thread B: now_A = getsbinuptime(); now_B = getsbinuptime(); // now_B > now_A RANDOM_RESEED_LOCK(); if (now - fs_lasttime > SBT_1S/10) { fs_lasttime = now; ... // reseed } RANDOM_RESEED_UNLOCK(); RANDOM_RESEED_LOCK(); if (now_A - fs_lasttime > SBT_1S/10) // now_A - fs_lasttime underflows fs_lasttime = now_A; ... // reseed again, despite less than 100ms elapsing } RANDOM_RESEED_UNLOCK(); To resolve the race, simply check the current time after we win the lock race. If getsbinuptime is perceived to be expensive, another option might be to just accept the race and validate that fs_lasttime isn't "in the future." (It should be within the last ~2^31 seconds out of ~2^32 seconds representable duration.) Reviewed by: delphij, markm Approved by: secteam (delphij) Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D16984 --- sys/dev/random/fortuna.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/dev/random/fortuna.c b/sys/dev/random/fortuna.c index 6382d47b1415..f9a29fd5a596 100644 --- a/sys/dev/random/fortuna.c +++ b/sys/dev/random/fortuna.c @@ -368,11 +368,11 @@ random_fortuna_pre_read(void) u_int i; KASSERT(fortuna_state.fs_minpoolsize > 0, ("random: Fortuna threshold must be > 0")); + RANDOM_RESEED_LOCK(); #ifdef _KERNEL /* FS&K - Use 'getsbinuptime()' to prevent reseed-spamming. */ now = getsbinuptime(); #endif - RANDOM_RESEED_LOCK(); if (fortuna_state.fs_pool[0].fsp_length >= fortuna_state.fs_minpoolsize #ifdef _KERNEL