rx: update_nextCid overflow handling is broken

The overflow handling in update_nextCid() produces a rx_nextCid
value of 0x80000001 which itself is out of the valid range.   When
used to construct the first call of a new connection the connection
id for the call becomes 0x80000002, and all subsequent connections
also trigger the overflow handling and thus also receive connection
id 0x80000002.

If the same connection id is used for multiple connections from
the same endpoint the accepting rx peer will be very confused.

When authenticated connections are used, the CHALLENGE/RESPONSE
will fail because of a mismatch in the connection's callNumber
array.

If an initiator makes only a single connection to a given rx peer,
that connection would succeed, but once multiple connections are
initiated all communication from a broken initiator to any rx peer
will fail.

The incorrect overflow calculation was introduced by
39b165cdda941181845022c183fea1c7af7e4356 ("Move epoch and cid
generation into the rx core").

This change corrects the overflow value to become

  1 << RX_CIDSHIFT

Change-Id: If36e3aa581d557cc0f4d2d478f84a6593224c3cc
Reviewed-on: https://gerrit.openafs.org/14492
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Jeffrey Altman 2021-01-14 09:57:13 -05:00 committed by Benjamin Kaduk
parent a3bc7ff150
commit 2c0a3901cb

View File

@ -6795,9 +6795,8 @@ update_nextCid(void)
{
/* Overflow is technically undefined behavior; avoid it. */
if (rx_nextCid > MAX_AFS_INT32 - (1 << RX_CIDSHIFT))
rx_nextCid = -1 * ((MAX_AFS_INT32 / RX_CIDSHIFT) * RX_CIDSHIFT);
else
rx_nextCid += 1 << RX_CIDSHIFT;
rx_nextCid = 0;
rx_nextCid += 1 << RX_CIDSHIFT;
}
static void