Page MenuHomeFreeBSD

Fortuna: Fix a race to prevent reseed spamming
ClosedPublic

Authored by cem on Sep 1 2018, 5:07 AM.

Details

Summary

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.)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem created this revision.Sep 1 2018, 5:07 AM
delphij accepted this revision.Sep 1 2018, 5:42 AM
delphij added a subscriber: delphij.

This is reasonable.

sys/dev/random/fortuna.c
413 ↗(On Diff #47556)

Unrelated to this change, but these shouldn't be done with lock held.

This revision is now accepted and ready to land.Sep 1 2018, 5:42 AM
markm accepted this revision.Sep 1 2018, 10:10 AM
This revision was automatically updated to reflect the committed changes.