From: Jeffrey Altman Date: Thu, 7 Jun 2018 01:23:14 +0000 (-0400) Subject: rx: reset packet header userStatus field on reuse X-Git-Tag: upstream/1.8.3^2~19 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=af0636ac6f69a6f82f4fe7ab1895f4559a55c34b;p=packages%2Fo%2Fopenafs.git rx: reset packet header userStatus field on reuse OpenAFS Rx fails to set the rx packet header userStatus field for most packets sent other than type RX_PACKET_TYPE_ACK. If the userStatus field is not set, its value will be random garbage based upon the prior use of the memory allocated to the rx_packet. This change explicitly sets the userStatus field to zero for all DATA and Special packet types. Background ---------- OpenAFS Rx allocates a pool of rx_packet structures that are reused for both incoming and outgoing Rx packets throughout the lifetime of the process (or kernel module). The rx packet header field userStatus is set by rxi_Send() to rx_call.localStatus. rxi_Send() is called from both rxi_SendAck() when sending RX_PACKET_TYPE_ACK packets and from rxi_SendSpecial() when called with a non-NULL call structure (RX_PACKET_TYPE_BUSY, RX_PACKET_TYPE_ACKALL, or RX_PACKET_TYPE_ABORT). rx_call.localStatus defaults to zero and can be modified by the application calling rx_SetLocalStatus(). The userStatus field is neither set nor reset when sending RX_PACKET_TYPE_DATA packets and all packets sent without a call structure. When allocated packets are reused in these cases, the value of the userStatus leaks from the prior packet use. The userStatus field is expected to be zero unless intentionally set by the application protocol to another value. The AFS3 suite of rx services uses the rx_header.userStatus field only in the RXAFS service and only as part of the definition for RXAFS_StoreData and RXAFS_StoreData64 RPCs. The StoreData RPCs use the rx_header.userStatus field as an out-of-band communication mechanism that permits the fileserver to signal to the cache manager when the RXAFS_StoreData[64] has been assigned to an application worker (thread) and the worker has acquired all of the required locks and other resources necessary to complete the RPC. This signal can be sent before all of the application data has been received. The cache manager reads the userStatus value via rx_GetRemoteStatus(). When bit-0 of the remote status value equals one and CSafeStore mode is disabled, the cache manager can wakeup any threads blocked waiting for the store operation to complete. Cache managers that perform a workload heavy in RXAFS_StoreData[64] RPCs will end up with an increasing percentage of packets in which the userStatus field is one instead of zero. Fileservers processing a workload heavy in RXAFS_StoreData[64] RPCs will likewise end up with an increasing percentage of packets in which the userStatus field is one instead of zero. Cache managers and Fileservers will therefore send DATA and call free special packets with a non-zero userStatus field to peer services (RXAFS, RXAFSCB, VL, PR). The failure to reset the userStatus field has not been a problem in the past because only the OpenAFS cache manager has ever queried the userStatus via rx_GetRemoteStatus() and only when issuing RXAFS_StoreData[64] RPCs. Failure to correct this flaw interferes with future use of the userStatus field in yet to be registered AFS3 RPCs and existing non-AFS3 services that make use of the userStatus when sending data to a service. FIXES: 134554 Reviewed-on: https://gerrit.openafs.org/13165 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk (cherry picked from commit c553170bcf3b97ba3745f21040c8e07b128ef983) Change-Id: I4e3c7fea876225ec401988a16b21ed3bb0760ee0 Reviewed-on: https://gerrit.openafs.org/13332 Reviewed-by: Michael Meffie Reviewed-by: Mark Vitale Reviewed-by: Cheyenne Wills Tested-by: BuildBot Reviewed-by: Stephan Wiesand --- diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c index 7c9551595..9340da3f7 100644 --- a/src/rx/rx_packet.c +++ b/src/rx/rx_packet.c @@ -1598,6 +1598,7 @@ rxi_SplitJumboPacket(struct rx_packet *p, afs_uint32 host, short port, np->header = p->header; np->header.serial = p->header.serial + 1; np->header.seq = p->header.seq + 1; + np->header.userStatus = 0; np->header.flags = jp->flags; np->header.spare = jp->cksum; @@ -2645,6 +2646,7 @@ rxi_SendSpecial(struct rx_call *call, p->header.seq = 0; p->header.epoch = conn->epoch; p->header.type = type; + p->header.userStatus = 0; p->header.flags = 0; if (conn->type == RX_CLIENT_CONNECTION) p->header.flags |= RX_CLIENT_INITIATED; @@ -2767,6 +2769,7 @@ rxi_PrepareSendPacket(struct rx_call *call, p->header.seq = seq; p->header.epoch = conn->epoch; p->header.type = RX_PACKET_TYPE_DATA; + p->header.userStatus = 0; p->header.flags = 0; p->header.spare = 0; if (conn->type == RX_CLIENT_CONNECTION)