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
This commit is contained in:
Conrad Meyer 2018-10-20 21:09:12 +00:00
parent 93d587e4d0
commit 5528565a76
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=339492

View File

@ -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