rx: reset packet header userStatus field on reuse 65/13165/6
authorJeffrey Altman <jaltman@auristor.com>
Thu, 7 Jun 2018 01:23:14 +0000 (21:23 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 14 Sep 2018 02:37:32 +0000 (22:37 -0400)
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.

Change-Id: I32c0bba93b8e5197036d92168956b6e2a95406fb
FIXES: 134554
Reviewed-on: https://gerrit.openafs.org/13165
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/rx/rx_packet.c

index 7c95515..9340da3 100644 (file)
@@ -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)