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