rx: Avoid new server calls for non-DATA packets 58/13758/10
authorMark Vitale <mvitale@sinenomine.net>
Thu, 8 Aug 2019 22:18:22 +0000 (18:18 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Tue, 29 Dec 2020 20:11:23 +0000 (15:11 -0500)
Normally, a client starts a new Rx call by sending DATA packets for
that call to a server, and rxi_ReceiveServerCall on the server creates
a new call struct for that call (since we don't recognize it as an
existing call).

Under certain circumstances, it's possible for a server to see a
non-DATA packet as the first packet for a call, and currently
rxi_ReceiveServerCall will create a new server call for any packet
type. The call cannot actually proceed until the server receives data
from the client (and goes through the challenge/response auth
handshake, if needed), but usually this is harmless, since the
existence of any packets for a particular call channel indicate that
the client is trying to run such a call. The server will respond to
the client with ACKs to indicate that it is missing the needed DATA
packet(s), and the client will send them and the call can proceed.

However, if a call is in the middle of running when the server is
restarted, the client may be sending ACKs for a pre-existing call that
the server doesn't know about. In this case, the server generates ACKs
that indicate the server has not received any DATA packets, which may
appear to violate the protocol, depending on the prior state of the
call (e.g. the server appears to try to move the window backwards).

Clients should be able to detect this and kill the call, but many do
not. For many OpenAFS releases before commit 7b204946 (rx: Avoid
lastReceiveTime update for invalid ACKs), the client will get confused
in this situation and will keep the call open forever, never making
progress.

There isn't any benefit to creating a new server call in these
situations, so just ignore non-DATA packets for unrecognized calls, to
avoid stalled calls from such clients. Those clients will not get a
response from the server, and so the call will eventually die from the
normal Rx call timeout.

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

src/rx/rx.c

index b008400..06c9239 100644 (file)
@@ -3294,6 +3294,22 @@ rxi_ReceiveServerCall(osi_socket socket, struct rx_packet *np,
     call = conn->call[channel];
 
     if (!call) {
+       if (np->header.type != RX_PACKET_TYPE_DATA) {
+           /*
+            * Clients must send DATA packets at some point to create a new
+            * call. If the first packet we saw for this call channel is
+            * something else, then either the DATA packets got lost/delayed,
+            * or we were restarted and this is an existing call from before we
+            * were restarted. In the latter case, some clients get confused if
+            * we respond to such requests, so just drop the packet to make
+            * things easier for them.
+            */
+           MUTEX_EXIT(&conn->conn_call_lock);
+           if (rx_stats_active)
+               rx_atomic_inc(&rx_stats.spuriousPacketsRead);
+           return NULL;
+       }
+
        if (rxi_AbortIfServerBusy(socket, conn, np)) {
            MUTEX_EXIT(&conn->conn_call_lock);
            return NULL;