jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-18 06:50:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

OPENAFS-SA-2019-003: ubik: Avoid unlocked ubik_currentTrans deref

Browse Source 
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 6ec46ba777)

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 213b9dc386)

Change-Id: I0ddbc2309de09a629150a6b3825e1aa8bda8b910
Reviewed-on: https://gerrit.openafs.org/13923
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2019-09-16 14:06:53 -05:00 committed by Benjamin Kaduk
parent 86e20c72f6
commit 1392a35879
1 changed files with 2 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/ubik/vote.c
View File

@ -451,10 +451,7 @@ SVOTE_Debug(struct rx_call * rxcall, struct ubik_debug * aparm)
if (ubik_currentTrans) {
aparm->currentTrans = 1;
if (ubik_currentTrans->type == UBIK_WRITETRANS)
aparm->writeTrans = 1;
else
aparm->writeTrans = 0;
aparm->writeTrans = 1;
} else {
aparm->currentTrans = 0;
}
@ -534,10 +531,7 @@ SVOTE_DebugOld(struct rx_call * rxcall,
if (ubik_currentTrans) {
aparm->currentTrans = 1;
if (ubik_currentTrans->type == UBIK_WRITETRANS)
aparm->writeTrans = 1;
else
aparm->writeTrans = 0;
aparm->writeTrans = 1;
} else {
aparm->currentTrans = 0;
}