OpenAFS Master Repository branch, master, updated. openafs-devel-1_9_1-429-g9d7b944
Gerrit Code Review
gerrit@openafs.org
Wed, 3 Jul 2024 14:25:30 -0400
The following commit has been merged in the master branch:
commit 9d7b94493c3d0230c15417436885a4211caeb411
Author: Andrew Deason <adeason@sinenomine.net>
Date: Thu Oct 20 18:31:43 2022 -0500
rx: Use atomics for rx_securityClass refcounts
Currently, the refCount in struct rx_securityClass is not protected by
any locks. Thus, if two threads create or destroy a connection using
the same rx_securityClass at the same time (or call rxs_Release), the
refCount can become inaccurate. If the refCount is undercounted, we
can prematurely free it while it's still referenced by other
connections or services, leading to segfaults, data corruption, etc.
For client connections, this can happen between any threads that
create and destroy a connection using the same security class struct.
For server connections, only two threads can race in this way: the rx
listener thread (which creates connections), and the rx event thread
(which destroys idle connections in rxi_ReapConnections).
To fix this, ideally we would change the refCount field to be an
rx_atomic_t. However, struct rx_securityClass is declared in the
public installed rx.h header, which cannot include rx_atomic.h. So
instead, change refCount users to go through a few new functions:
rxs_Ref(), rxs_DecRef(), and rxs_SetRefs(). These functions interpret
the refCount as an rx_atomic_t, and so allows callers to use safe
refcounting without needing to call rx_atomic_* functions directly.
Rename the existing refCount field to refCount_data, and declare it as
a char[8]. This gives us enough space to use it as an rx_atomic_t, but
avoids using rx_atomic_t in a public header, and discourages callers
from manipulating the refCount directly.
Thanks to mvitale@sinenomine.net for helping investigate the relevant
issue.
Change-Id: I55094218c79e8bc5498a6d2c1daa5620b1fceaff
Reviewed-on: https://gerrit.openafs.org/15158
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
src/rx/rx.c | 68 +++++++++++++++++++++++++++++++++++++++++-------
src/rx/rx.h | 8 +++++-
src/rx/rx_null.c | 2 +-
src/rx/rx_prototypes.h | 3 +++
src/rxgk/rxgk_client.c | 16 +++---------
src/rxgk/rxgk_server.c | 16 +++---------
src/rxkad/rxkad_client.c | 2 +-
src/rxkad/rxkad_common.c | 11 +++-----
src/rxkad/rxkad_server.c | 2 +-
9 files changed, 82 insertions(+), 46 deletions(-)
--
OpenAFS Master Repository