OpenAFS Master Repository branch, master, updated. openafs-devel-1_9_2-129-gbe65c9c

[Gerrit Code Review]

The following commit has been merged in the master branch:
commit be65c9c107364945dae0b7cb7ffe27e0f359ff0d
Author: Mark Vitale <mvitale@sinenomine.net>
Date:   Tue Apr 23 21:19:59 2024 -0400

    rx: Lock rx_packets_mutex for rx_TSFPQ* globals
    
    The following global variables are documented (src/rx/rx.c) as protected
    by rx_packets_mutex:
    
      rx_TSFPQGlobSize
      rx_TSFPQLocalMax
      rx_TSFPQMaxProcs
      rx_nPackets
    
    However, the current implementation has a number of instances where
    these variables are either modified or read without holding
    rx_packets_mutex.
    
    For example, rx_TSFPQGlobSize is modified in RX_TS_FPQ_COMPUTE_LIMITS,
    and all callers of this macro do so while holding rx_packets_mutex.
    However, several other users read rx_TSFPQGlobSize without holding
    rx_packets_mutex: RX_TS_FPQ_LTOG, RX_TS_FPQ_GTOL, and AllocPacketBufs.
    
    In particular (since commit 35285dad3e rx-fpq-optimize-20050425),
    RX_TS_FPQ_GTOL examines rx_TSFPQGlobSize twice in quick succession
    without holding rx_packets_mutex.  First it checks if rx_TSFPQGlobSize
    is less than rx_nFreePackets.  If so, it reads rx_TSFPQGlobSize again
    to assign its value to tsize.  But because we're not holding
    rx_packets_mutex, rx_TSFPQGlobSize may have been modified in another
    thread to be larger than nFreePackets.  This results in a tsize that
    will attempt to remove more packets from the global free-packet queue
    than are actually present, corrupting the queue and leading to an
    eventual segfault.
    
    Ensure that rx_packets_mutex is held for all[1] reads and writes of
    rx_TSFPQGlobSize, rx_TSFPQLocalMax, rx_TSFPQMaxProcs, and rx_nPackets.
    Ensure that we hold rx_packets_mutex from time-of-check through
    time-of-use in RX_TS_FPQ_GTOL. Ensure that when both rx_packets_mutex
    and rx_freePktQ_lock are required, rx_freePktQ_lock is always held
    first in accordance with the documented locking order in src/rx/rx.c.
    
    Add a new function, rxi_ts_fpq_overquota(), to make the locking easier
    for a few places in rx_packet.c, and remove a nearby outdated comment
    about continuation buffers that now looks even more confusing.
    
    [1] We forgo locking for rx_nPackets during initialization if it occurs
    before any other threads (e.g., rx_Listener) could be using this global:
    
      - rx_InitHost()
      - main() in src/rx/test/testclient.c
      - main() in src/volser/volmain.c (volserver)
    
    We also forgo locking for rx_nPackets in rxi_ReceiveDebugPacket() to quickly
    gather statistics without blocking, at the slight risk of occasional
    inaccuracy.
    
    Change-Id: I84c5b3885b47c1708965ee6ff96e9867ba2007b4
    Reviewed-on: https://gerrit.openafs.org/15878
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>

 src/rx/rx_globals.h |  7 ++++++-
 src/rx/rx_packet.c  | 37 ++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 8 deletions(-)

-- 
OpenAFS Master Repository