OpenAFS Master Repository branch, openafs-stable-1_6_x, updated. openafs-stable-1_6_24-92-g304a589

Gerrit Code Review gerrit@openafs.org
Tue, 22 Oct 2019 19:24:23 -0400


The following commit has been merged in the openafs-stable-1_6_x branch:
commit 3915886843a1d4b21bd2539e7e9d4057965a52dd
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Mon Sep 16 14:06:53 2019 -0500

    OPENAFS-SA-2019-003: ubik: Avoid unlocked ubik_currentTrans deref
    
    Currently, SVOTE_Debug/SVOTE_DebugOld examine some ubik internal state
    without any locks, because the speed of these functions is more
    important than accuracy.
    
    However, one of the pieces of data we examine is ubik_currentTrans,
    which we dereference to get ubik_currentTrans->type. ubik_currentTrans
    could be set to NULL while this code is running, so there is a small
    chance of this code causing a segfault, if SVOTE_Debug() is running
    when the current transaction ends.
    
    We only ever initialize ubik_currentTrans as a write transation (via
    SDISK_Begin), so this check is pointless anyway. Accordingly, skip the
    type check, and always assume that any active transaction is a write
    transaction.  This means we only ever access ubik_currentTrans once,
    avoiding any risk of the value changing between accesses (and we no
    longer need to dereference it, anyway).
    
    Note that, since ubik_currentTrans is not marked as 'volatile', some C
    compilers, with certain options, can and do assume that its value will
    not change between accesses, and thus only fetch the pointer value once.
    This avoids the risk of NULL dereference (and thus, crash, if pointer
    stores/loads are atomic), but the value pointed to by ubik_currentTrans->type
    would be incorrect when the transaction ends during the execution of
    SVOTE_Debug().
    
    Reviewed-on: https://gerrit.openafs.org/13915
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 6ec46ba7773089e1549d27a0d345afeca65c9472)
    
    Reviewed-on: https://gerrit.openafs.org/13918
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 213b9dc386ff89a14379313ee8ec09280f130a29)
    
    Change-Id: I0ddbc2309de09a629150a6b3825e1aa8bda8b910
    Reviewed-on: https://gerrit.openafs.org/13923
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

 src/ubik/vote.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

-- 
OpenAFS Master Repository