rx: race in rx_multi processing
authorJeffrey Altman <jaltman@your-file-system.com>
Tue, 28 Jun 2011 13:35:02 +0000 (09:35 -0400)
committerDerrick Brashear <shadow@dementia.org>
Tue, 28 Jun 2011 19:51:44 +0000 (12:51 -0700)
multi_Init() registers an arrival procedure which is called when
the first response packet for the call arrives.  If the call times
out the multi_Body loop will call rx_EndCall() and then set
multi_h->calls[multi_i] to NULL.  If the first data packet of the
call arrives before rx_EndCall() is executed, then the arrival
procedure, multi_Ready(), will be executed adding the call to the
firstNotReady list.  When the multi_Body loop attempts to process
the call from the firstNotReady list it attempts to dereference
the NULL multi_call.  This race was introduced by
be4abb4ec83a47477b254f2b3375742c4efbb063.

multi_h->calls[multi_i] is set to NULL as an indicator to
multi_Finalize() that rx_EndCall() has already been processed
for the call.  When rx_EndCall() is executed the arrival
procedure is cleared.

If rx_EndCall() has already been processed, the fact that
the arrival procedure has been executed must be ignored.  Add
an additional check in multi_Body for a non-NULL call pointer
to skip the startProc and rx_FlushWrite processing on the
no longer existent call.

Note that it is not safe to hold onto the call reference after
rx_EndCall() has been processed since the call slot may be
reused for a new RPC before the multi processing on all calls
is complete.

Change-Id: Ib4694a7e1d133f621d15e79534a42f780b141e34
Reviewed-on: http://gerrit.openafs.org/4890
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

src/rx/rx_multi.h

index ad1c264..de67ae3 100644 (file)
@@ -36,8 +36,10 @@ struct multi_handle {
 #define multi_Body(startProc, endProc)\
        if (multi_h->nextReady == multi_h->firstNotReady && multi_i < multi_h->nConns) {\
            multi_call = multi_h->calls[multi_i];\
-           startProc;\
-           rx_FlushWrite(multi_call);\
+            if (multi_call) {\
+                startProc;\
+               rx_FlushWrite(multi_call);\
+            }\
            multi_i0++;  /* THIS is the loop variable!! */\
            continue;\
        }\