OpenAFS Master Repository branch, openafs-stable-1_8_x, updated. openafs-stable-1_8_3-22-ge80e9e6

Gerrit Code Review gerrit@openafs.org
Mon, 10 Jun 2019 07:55:24 -0400


The following commit has been merged in the openafs-stable-1_8_x branch:
commit e80e9e6ea6f2cedf79b92ea33eb214931bf808be
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Sat Mar 2 15:58:00 2019 -0600

    afs: Cleanup state on rxfs_*Init errors
    
    Currently, rxfs_storeInit and rxfs_fetchInit return early if they
    encounter an error while starting the relevant fetch/store RPC (e.g.
    StartRXAFS_FetchData64). In this scenario, they osi_FreeSmallSpace
    their rock before returning, but they never go through their
    destructor to free the contents of the rock
    (rxfs_storeDestroy/rxfs_fetchDestroy), leaking any resources inside
    that have already been initialized.
    
    The only thing that could have been initialized by this point is
    v->call, so hitting this condition means we leak an Rx call, and means
    we can report the wrong error code (since we never go through
    rx_EndCall, we never look at the call's abort code). For
    rxfs_fetchInit, most code paths call rx_EndCall explicitly, except for
    the code path where StartRXAFS_FetchData64 itself fails.
    
    For both fetches and stores, it's difficult to hit this condition,
    because this requires that the StartRXAFS_* call fails, before we have
    sent or received any data from the wire. However, this can be hit if
    the call is already aborted before we use it, which can happen if the
    underlying connection has already been aborted by a connection abort.
    
    Before commit 0835d7c2 ("afs: make sure to call afs_Analyze after
    afs_Conn"), this was most easily hit by trying to fetch data with a
    bad security object (for example, with expired credentials). After the
    first fetch failed due to a connection abort (e.g. RXKADEXPIRED),
    afs_GetDCache would retry the fetch with the same connection, and
    StartRXAFS_FetchData64 would fail because the connection and call were
    already aborted. In this case, we'd leak the Rx call, and we would
    throw an RXGEN_CC_MARSHAL error (-450), instead of the correct
    RXKADEXPIRED error. This causes libafs to report that the target
    server as unreachable, due to the negative error code.
    
    With commit 0835d7c2, this doesn't happen because we call afs_Analyze
    before retrying the fetch, which detects the invalid credentials and
    forces creating a new connetion object. However, this situation should
    still be possible if a different call on the same connection triggered
    a connection-level abort before we called StartRXAFS_FetchData64.
    
    To fix this and ensure that we don't leak Rx calls, explicitly call
    rxfs_storeDestroy/rxfs_fetchDestroy in this error case, before
    returning from rxfs_storeInit/rxfs_fetchInit.
    
    Thanks to yadayada@in.ibm.com for reporting a related issue and
    providing analysis.
    
    Reviewed-on: https://gerrit.openafs.org/13510
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit 11cc0a3c4e0d76f1650596bd1568f01367ab5be2)
    
    Change-Id: I3c2d66a5a6128bb8b403dfa6ea7c37e32bd2f156
    Reviewed-on: https://gerrit.openafs.org/13517
    Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
    Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
    Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

 src/afs/afs_fetchstore.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

-- 
OpenAFS Master Repository