Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 6 Aug 2021 05:13:32 GMT
From:      "David E. O'Brien" <obrien@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: ba2f52819c51 - stable/12 - Fortuna: Fix a race to prevent reseed spamming
Message-ID:  <202108060513.1765DW3I000868@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by obrien:

URL: https://cgit.FreeBSD.org/src/commit/?id=ba2f52819c51853ce9f158878f13da736718fad2

commit ba2f52819c51853ce9f158878f13da736718fad2
Author:     Conrad Meyer <cem@FreeBSD.org>
AuthorDate: 2018-10-20 21:09:12 +0000
Commit:     David E. O'Brien <obrien@FreeBSD.org>
CommitDate: 2021-08-06 05:12:00 +0000

    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
    (cherry picked from commit 5528565a76f5caae336d4f13213108dc1fad4ae0)
---
 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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202108060513.1765DW3I000868>