rxevent: prevent negative rx_connection refCount

rxi_ChallengeEvent is called directly from rxi_ChallengeOn to start the
first challenge; subsequent calls to rxi_ChallengeEvent are from the
event handler.  When called as an event, we must putConnection the
reference held by the event.  But when called directly for the first
time, the event has not been scheduled yet and so has not taken a
reference on the connection.  For this case, we must not putConnection
or the rx_connection refCount will go negative.

One reported symptom of this bug is a fileserver crash with:
  'Assertion failed! file rx.c, line 1327.'

Introduced by commit 304d758983b499dc568d6ca57b6e92df24b69de8
('Standardize rx_event usage').

Reviewed-on: https://gerrit.openafs.org/13228
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit b1ad473be01162fe9b3835544a835c4dcf0fcb35)

Change-Id: Ice5856627f4f77b5ede3a84fef4b6f2915f5477d
Reviewed-on: https://gerrit.openafs.org/13229
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index e84cf32de49c9ec711594703618699e1c6f40f7e..651136a2afbfa3eed7a60111ff2d89254a3304d7 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -6702,27 +6702,30 @@ rxi_SendDelayedCallAbort(struct rxevent *event, void *arg1, void *dummy,
  *
  * This routine is both an event handler and a function called directly;
  * when called directly the passed |event| is NULL and the
- * conn->conn->data>lock must must not be held.
+ * conn->conn->data>lock must must not be held.  Also, when called as an
+ * an event handler, we must putConnection before we exit; but when called
+ * directly (the first challenge), we must NOT putConnection.
  */
 static void
 rxi_ChallengeEvent(struct rxevent *event,
                   void *arg0, void *arg1, int tries)
 {
     struct rx_connection *conn = arg0;
+    int event_raised = 0;      /* assume we were called directly */
 
     MUTEX_ENTER(&conn->conn_data_lock);
-    if (event != NULL && event == conn->challengeEvent)
+    if (event != NULL && event == conn->challengeEvent) {
+       event_raised = 1;       /* called as an event */
        rxevent_Put(&conn->challengeEvent);
+    }
     MUTEX_EXIT(&conn->conn_data_lock);
 
     /* If there are no active calls it is not worth re-issuing the
      * challenge.  If the client issues another call on this connection
      * the challenge can be requested at that time.
      */
-    if (!rxi_HasActiveCalls(conn)) {
-       putConnection(conn);
-        return;
-    }
+    if (!rxi_HasActiveCalls(conn))
+       goto done;
 
     if (RXS_CheckAuthentication(conn->securityObject, conn) != 0) {
        struct rx_packet *packet;
@@ -6748,8 +6751,7 @@ rxi_ChallengeEvent(struct rxevent *event,
                }
            }
            MUTEX_EXIT(&conn->conn_call_lock);
-           putConnection(conn);
-           return;
+           goto done;
        }
 
        packet = rxi_AllocPacket(RX_PACKET_CLASS_SPECIAL);
@@ -6774,7 +6776,9 @@ rxi_ChallengeEvent(struct rxevent *event,
        }
        MUTEX_EXIT(&conn->conn_data_lock);
     }
-    putConnection(conn);
+ done:
+    if (event_raised)
+       putConnection(conn);
 }
 
 /* Call this routine to start requesting the client to authenticate