From bc54d67ed892ba17213d47cab59ac1437aa001ce Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Fri, 21 Feb 2025 21:48:34 -0800
Subject: [PATCH] [mle] clamp child timeout to min/max range (#11278)

This commit updates `SetTimeout()` to ensure the child timeout remains
within the minimum and maximum allowed values (per the specification,
the maximum value of 8 hours is used).

These limits are also enforced when the parent requests a different
timeout value in an MLE Child Update Response to the child's
request.
---
 src/core/common/time.hpp | 17 +++++++++++------
 src/core/thread/mle.cpp  | 17 ++++++++++++-----
 src/core/thread/mle.hpp  |  9 ++++++++-
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/src/core/common/time.hpp b/src/core/common/time.hpp
index 2119ec83b..70722185b 100644
--- a/src/core/common/time.hpp
+++ b/src/core/common/time.hpp
@@ -59,12 +59,17 @@ namespace ot {
 class Time : public Unequatable<Time>
 {
 public:
-    static constexpr uint32_t kOneSecondInMsec = 1000u;                 ///< One second interval in msec.
-    static constexpr uint32_t kOneMinuteInMsec = kOneSecondInMsec * 60; ///< One minute interval in msec.
-    static constexpr uint32_t kOneHourInMsec   = kOneMinuteInMsec * 60; ///< One hour interval in msec.
-    static constexpr uint32_t kOneDayInMsec    = kOneHourInMsec * 24;   ///< One day interval in msec.
-    static constexpr uint32_t kOneMsecInUsec   = 1000u;                 ///< One millisecond in microseconds.
-    static constexpr uint32_t kOneSecondInUsec = 1000000u;              ///< One second interval in microseconds.
+    static constexpr uint32_t kOneMinuteInSec = 60;                   ///< One minute interval in sec.
+    static constexpr uint32_t kOneHourInSec   = 60 * kOneMinuteInSec; ///< One hour interval in sec.
+    static constexpr uint32_t kOneDayInSec    = 24 * kOneHourInSec;   ///< One day interval in sec.
+
+    static constexpr uint32_t kOneSecondInMsec = 1000u;                              ///< One second interval in msec.
+    static constexpr uint32_t kOneMinuteInMsec = kOneMinuteInSec * kOneSecondInMsec; ///< One minute interval in msec.
+    static constexpr uint32_t kOneHourInMsec   = kOneHourInSec * kOneSecondInMsec;   ///< One hour interval in msec.
+    static constexpr uint32_t kOneDayInMsec    = kOneDayInSec * kOneSecondInMsec;    ///< One day interval in msec.
+
+    static constexpr uint32_t kOneMsecInUsec   = 1000u;    ///< One millisecond in microseconds.
+    static constexpr uint32_t kOneSecondInUsec = 1000000u; ///< One second interval in microseconds.
 
     /**
      * This constant defines a maximum time duration ensured to be longer than any other duration.
diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp
index 5aaaddf88..9ab034f05 100644
--- a/src/core/thread/mle.cpp
+++ b/src/core/thread/mle.cpp
@@ -791,16 +791,23 @@ exit:
     return;
 }
 
-void Mle::SetTimeout(uint32_t aTimeout)
+void Mle::SetTimeout(uint32_t aTimeout, TimeoutAction aAction)
 {
-    // Determine `kMinTimeout` based on other parameters
+    // Determine `kMinTimeout` based on other parameters. `kMaxTimeout`
+    // is set (per spec) to minimum Delay Timer value for a Pending
+    // Operational Dataset when updating the Network Key which is 8
+    // hours.
+
     static constexpr uint32_t kMinPollPeriod       = OPENTHREAD_CONFIG_MAC_MINIMUM_POLL_PERIOD;
     static constexpr uint32_t kRetxPollPeriod      = OPENTHREAD_CONFIG_MAC_RETX_POLL_PERIOD;
     static constexpr uint32_t kMinTimeoutDataPoll  = kMinPollPeriod + kFailedChildTransmissions * kRetxPollPeriod;
     static constexpr uint32_t kMinTimeoutKeepAlive = (kMaxChildKeepAliveAttempts + 1) * kUnicastRetxDelay;
     static constexpr uint32_t kMinTimeout          = Time::MsecToSec(OT_MAX(kMinTimeoutKeepAlive, kMinTimeoutDataPoll));
+    static constexpr uint32_t kMaxTimeout          = (8 * Time::kOneHourInSec);
 
-    aTimeout = Max(aTimeout, kMinTimeout);
+    static_assert(kMinTimeout <= kMaxTimeout, "Min timeout MUST be less than Max timeout");
+
+    aTimeout = Clamp(aTimeout, kMinTimeout, kMaxTimeout);
 
     VerifyOrExit(mTimeout != aTimeout);
 
@@ -808,7 +815,7 @@ void Mle::SetTimeout(uint32_t aTimeout)
 
     Get<DataPollSender>().RecalculatePollPeriod();
 
-    if (IsChild())
+    if (IsChild() && (aAction == kSendChildUpdateToParent))
     {
         IgnoreError(SendChildUpdateRequestToParent());
     }
@@ -3578,7 +3585,7 @@ void Mle::HandleChildUpdateResponseOnChild(RxInfo &aRxInfo)
             }
             else
             {
-                mTimeout = timeout;
+                SetTimeout(timeout, kDoNotSendChildUpdateToParent);
             }
             break;
         case kErrorNotFound:
diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp
index cf10c2bdc..25683c168 100644
--- a/src/core/thread/mle.hpp
+++ b/src/core/thread/mle.hpp
@@ -472,7 +472,7 @@ public:
      *
      * @param[in]  aTimeout  The Timeout value in seconds.
      */
-    void SetTimeout(uint32_t aTimeout);
+    void SetTimeout(uint32_t aTimeout) { SetTimeout(aTimeout, kSendChildUpdateToParent); }
 
     /**
      * Returns the RLOC16 assigned to the Thread interface.
@@ -904,6 +904,12 @@ private:
         kNoSecurity  = 255, // Security suite value indicating that MLE message is secured.
     };
 
+    enum TimeoutAction : uint8_t // Used as input in `SetTimeout()` to determine whether or not to update the parent.
+    {
+        kSendChildUpdateToParent,
+        kDoNotSendChildUpdateToParent,
+    };
+
     enum MessageAction : uint8_t
     {
         kMessageSend,
@@ -1330,6 +1336,7 @@ private:
     void       SetStateChild(uint16_t aRloc16);
     void       SetLeaderData(uint32_t aPartitionId, uint8_t aWeighting, uint8_t aLeaderRouterId);
     void       SetLeaderData(const LeaderData &aLeaderData);
+    void       SetTimeout(uint32_t aTimeout, TimeoutAction aAction);
     void       InformPreviousChannel(void);
     bool       IsAnnounceAttach(void) const { return mAlternatePanId != Mac::kPanIdBroadcast; }
     void       ScheduleMessageTransmissionTimer(void);