openafs/ChangeLog

commit e4e37ae5645ddb851d2ab956e1075a81d4dcc8cb
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Tue Oct 22 00:11:09 2019 -0700

    Make OpenAFS 1.8.5
    
    Update version strings for the 1.8.5 release.
    
    Change-Id: I6e4e4b02b2ad7686027e983d63919cd3045fd2d4
    Reviewed-on: https://gerrit.openafs.org/13920
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 2007a59e87f5c5d3a4df66e253f58b2af15774fb
Author: Benjamin Kaduk <kaduk@mit.edu>
Date:   Tue Oct 22 00:08:36 2019 -0700

    Update NEWS for 1.8.5
    
    Release notes for the OpenAFS 1.8.5 security release.
    
    Change-Id: Idd44efa17c41a9fa4d2d3beddb294a1c24bdec9e
    Reviewed-on: https://gerrit.openafs.org/13919
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 213b9dc386ff89a14379313ee8ec09280f130a29
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Mon Sep 16 14:06:53 2019 -0500

    OPENAFS-SA-2019-003: ubik: Avoid unlocked ubik_currentTrans deref
    
    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 6ec46ba7773089e1549d27a0d345afeca65c9472)
    
    Change-Id: I634ddb27e7a8dbe5c9d1dacdc83070efa470b50b
    Reviewed-on: https://gerrit.openafs.org/13918
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit fcaac44f845d18d6fd5d2f3685db11118d8f8626
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Wed Aug 7 21:19:47 2019 -0500

    OPENAFS-SA-2019-002: Zero all server RPC args
    
    Currently, our server-side RPC argument-handling code generated from
    rxgen initializes complex arguments like so (for example, in
    _RXAFS_BulkStatus):
    
        AFSCBFids FidsArray;
        AFSBulkStats StatArray;
        AFSCBs CBArray;
        AFSVolSync Sync;
    
        FidsArray.AFSCBFids_val = 0;
        FidsArray.AFSCBFids_len = 0;
        CBArray.AFSCBs_val = 0;
        CBArray.AFSCBs_len = 0;
        StatArray.AFSBulkStats_val = 0;
        StatArray.AFSBulkStats_len = 0;
    
    This is done for any input or output arguments, but only for types we
    need to free afterwards (arrays, usually). We do not do this for
    simple types, like single flat structs. In the above example, we do
    this for the arrays FidsArray, StatArray, and CBArray, but 'Sync' is
    not initialized to anything.
    
    If some server RPC handlers never set a value for an output argument,
    this means we'll send uninitialized stack memory to our peer.
    Currently this can happen in, for example,
    MRXSTATS_RetrieveProcessRPCStats if 'rxi_monitor_processStats' is
    unset (specifically, the 'clock_sec' and 'clock_usec' arguments are
    never set when rx_enableProcessRPCStats() has not been called).
    
    To make sure we cannot send uninitialized data to our peer, change
    rxgen to instead 'memset(&arg, 0, sizeof(arg));' for every single
    parameter. Using memset in this way just makes this a little simpler
    inside rxgen, since all we need to do this is the name of the
    argument.
    
    With this commit, the rxgen-generated code for the above example now
    looks like this:
    
        AFSCBFids FidsArray;
        AFSBulkStats StatArray;
        AFSCBs CBArray;
        AFSVolSync Sync;
    
        memset(&FidsArray, 0, sizeof(FidsArray));
        memset(&CBArray, 0, sizeof(CBArray));
        memset(&StatArray, 0, sizeof(StatsArray));
        memset(&Sync, 0, sizeof(Sync));
    
    Reviewed-on: https://gerrit.openafs.org/13914
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    (cherry picked from commit 93aee3cf40622993b95bd1af77080a31670c24bb)
    
    Change-Id: I6e19aaea57e545455b65851d1bedade584e482f0
    Reviewed-on: https://gerrit.openafs.org/13917
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>

commit 5a3d1b62810fc8cc7b37a737b4f5f1912bc614f9
Author: Andrew Deason <adeason@sinenomine.net>
Date:   Wed Aug 7 20:50:47 2019 -0500

    OPENAFS-SA-2019-001: Skip server OUT args on error
    
    Currently, part of our server-side RPC argument-handling code that's
    generated from rxgen looks like this (for example):
    
        z_result = SRXAFS_BulkStatus(z_call, &FidsArray, &StatArray, &CBArray, &Sync);
        z_xdrs->x_op = XDR_ENCODE;
        if ((!xdr_AFSBulkStats(z_xdrs, &StatArray))
             || (!xdr_AFSCBs(z_xdrs, &CBArray))
             || (!xdr_AFSVolSync(z_xdrs, &Sync)))
                z_result = RXGEN_SS_MARSHAL;
    fail:
        [...]
        return z_result;
    
    When the server routine for implementing the RPC results a non-zero
    value into z_result, the call will be aborted. However, before we
    abort the call, we still call the xdr_* routines with XDR_ENCODE for
    all of our output arguments. If the call has not already been aborted
    for other reasons, we'll serialize the output argument data into the
    Rx call. If we push more data than can fit in a single Rx packet for
    the call, then we'll also send that data to the client. Many server
    routines for implementing RPCs do not initialize the memory inside
    their output arguments during certain errors, and so the memory may be
    leaked to the peer.
    
    To avoid this, just jump to the 'fail' label when a nonzero 'z_result'
    is returned. This means we skip sending the output argument data to
    the peer, but we still free any argument data that needs freeing, and
    record the stats for the call (if needed). This makes the above
    example now look like this:
    
        z_result = SRXAFS_BulkStatus(z_call, &FidsArray, &StatArray, &CBArray, &Sync);
        if (z_result)
            goto fail;
        z_xdrs->x_op = XDR_ENCODE;
        if ((!xdr_AFSBulkStats(z_xdrs, &StatArray))
             || (!xdr_AFSCBs(z_xdrs, &CBArray))
             || (!xdr_AFSVolSync(z_xdrs, &Sync)))
                z_result = RXGEN_SS_MARSHAL;
    fail:
        [...]
        return z_result;
    
    Reviewed-on: https://gerrit.openafs.org/13913
    Reviewed-by: Andrew Deason <adeason@sinenomine.net>
    Tested-by: BuildBot <buildbot@rampaginggeek.com>
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    (cherry picked from commit ea276e83e37e5bd27285a3d639f2158639172786)
    
    Change-Id: I688cbf1a65903bf26a0db033687898f3fb5a54ea
    Reviewed-on: https://gerrit.openafs.org/13916
    Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
    Tested-by: Benjamin Kaduk <kaduk@mit.edu>