From: Benjamin Kaduk Date: Thu, 14 Jan 2021 18:50:13 +0000 (-0800) Subject: Pull in upstream patches for unixtime 0x60000000 CID fix X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=d9798ac1a2287238903335bda06622a00b803b42;p=packages%2Fo%2Fopenafs.git Pull in upstream patches for unixtime 0x60000000 CID fix A combination of bugs led to a static CID value being used for all outbound connections when a process was started after unix epoch time 0x60000000. That essentially means failure to communicate on all connections. Fix the bugs, and also make sure that we always use a random initial CID instead of getting a static one half the time. Change-Id: I1d39822d6a32797bbb123fc78b4fc1f5f4c890ea --- diff --git a/debian/patches/0012-rx-rx_InitHost-do-not-overwrite-RAND_bytes-rx_nextCi.patch b/debian/patches/0012-rx-rx_InitHost-do-not-overwrite-RAND_bytes-rx_nextCi.patch new file mode 100644 index 000000000..2d2c79ac9 --- /dev/null +++ b/debian/patches/0012-rx-rx_InitHost-do-not-overwrite-RAND_bytes-rx_nextCi.patch @@ -0,0 +1,43 @@ +From: Jeffrey Altman +Date: Thu, 14 Jan 2021 09:41:39 -0500 +Subject: rx: rx_InitHost do not overwrite RAND_bytes rx_nextCid + +39b165cdda941181845022c183fea1c7af7e4356 ("Move epoch and cid +generation into the rx core") introduced the use of RAND_bytes() +to generate the initial 'rx_nextCid' but failed to remove the + + rx_nextCid = ((tv.tv_sec ^ tv.tv_usec) << RX_CIDSHIFT; + +assignment inherited from IBM/Transarc. + +At Thu, 14 Jan 2021 08:25:36 GMT the IBM inherited calculation +overflows the value CID range. This triggers broken overflow +logic in update_nextCid(). + +Change-Id: Ib7283def1ded9792d394133a3969a6d86f3a6123 +Reviewed-on: https://gerrit.openafs.org/14491 +Reviewed-by: Andrew Deason +Tested-by: Andrew Deason +Reviewed-by: Jeffrey Hutzelman +Reviewed-by: Cheyenne Wills +Tested-by: Mark Vitale +Reviewed-by: Benjamin Kaduk +(cherry picked from commit a3bc7ff1501d51ceb3b39d9caed62c530a804473) +--- + src/rx/rx.c | 3 --- + 1 file changed, 3 deletions(-) + +diff --git a/src/rx/rx.c b/src/rx/rx.c +index 244838d..e1e6d8f 100644 +--- a/src/rx/rx.c ++++ b/src/rx/rx.c +@@ -621,9 +621,6 @@ rx_InitHost(u_int host, u_int port) + MUTEX_ENTER(&rx_quota_mutex); + rxi_dataQuota += rx_extraQuota; /* + extra pkts caller asked to rsrv */ + MUTEX_EXIT(&rx_quota_mutex); +- /* *Slightly* random start time for the cid. This is just to help +- * out with the hashing function at the peer */ +- rx_nextCid = ((tv.tv_sec ^ tv.tv_usec) << RX_CIDSHIFT); + rx_connHashTable = (struct rx_connection **)htable; + rx_peerHashTable = (struct rx_peer **)ptable; + diff --git a/debian/patches/0013-rx-update_nextCid-overflow-handling-is-broken.patch b/debian/patches/0013-rx-update_nextCid-overflow-handling-is-broken.patch new file mode 100644 index 000000000..10053c2d2 --- /dev/null +++ b/debian/patches/0013-rx-update_nextCid-overflow-handling-is-broken.patch @@ -0,0 +1,57 @@ +From: Jeffrey Altman +Date: Thu, 14 Jan 2021 09:57:13 -0500 +Subject: 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 +Reviewed-by: Benjamin Kaduk +Tested-by: Benjamin Kaduk +(cherry picked from commit 2c0a3901cbfcb231b7b67eb0899a3133516f33c8) +--- + src/rx/rx.c | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/src/rx/rx.c b/src/rx/rx.c +index e1e6d8f..5d59531 100644 +--- a/src/rx/rx.c ++++ b/src/rx/rx.c +@@ -6651,9 +6651,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 diff --git a/debian/patches/0014-Remove-overflow-check-from-update_nextCid.patch b/debian/patches/0014-Remove-overflow-check-from-update_nextCid.patch new file mode 100644 index 000000000..d92986bec --- /dev/null +++ b/debian/patches/0014-Remove-overflow-check-from-update_nextCid.patch @@ -0,0 +1,32 @@ +From: Benjamin Kaduk +Date: Thu, 14 Jan 2021 10:20:59 -0800 +Subject: Remove overflow check from update_nextCid + +The rx_nextCid global has been an unsigned type since +http://gerrit.openafs.org/11106 (which was actually merged before +the refactoring of overflow check to avoid signed integer overflow) +and thus there is no need to avoid signed overflow. The per-connection +cid has been unsigned since the IBM import. + +The natural unsigned behavior on overflow of wrapping is the desired +behvaior here, so just remove the extra logic and always increment. + +Change-Id: I2d9fd24082b762eb871199da3ac1cc0983764585 +--- + src/rx/rx.c | 3 --- + 1 file changed, 3 deletions(-) + +diff --git a/src/rx/rx.c b/src/rx/rx.c +index 5d59531..9f2f821 100644 +--- a/src/rx/rx.c ++++ b/src/rx/rx.c +@@ -6649,9 +6649,6 @@ rxi_CancelGrowMTUEvent(struct rx_call *call) + static void + update_nextCid(void) + { +- /* Overflow is technically undefined behavior; avoid it. */ +- if (rx_nextCid > MAX_AFS_INT32 - (1 << RX_CIDSHIFT)) +- rx_nextCid = 0; + rx_nextCid += 1 << RX_CIDSHIFT; + } + diff --git a/debian/patches/series b/debian/patches/series index 851f3c32f..2e3b6f867 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,2 +1,5 @@ 0003-Catch-up-to-roken-s-rename-of-base64-symbols.patch butc-repair-build-error.patch +0012-rx-rx_InitHost-do-not-overwrite-RAND_bytes-rx_nextCi.patch +0013-rx-update_nextCid-overflow-handling-is-broken.patch +0014-Remove-overflow-check-from-update_nextCid.patch