From 631e9ba0b8ab1d8e0c79487e02696ebd2fb08436 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 31 Aug 2020 08:25:33 -0700
Subject: [PATCH] [sub-mac] do not assert when parsing raw-link frames (#5475)

This commit relaxes the assert checks when parsing raw-link frame from
the radio tx-done callback to update the security frame counter. In an
FTD/MTD build, if/when raw-link is enabled, the `TxFrame` is prepared
and given by user and may not necessarily follow 15.4 frame format
(raw-link can be used with vendor-specific format), so we allow
failure when parsing the frame. In other cases (in an RCP build or in
an FTD/MTD build without raw-link) since the `TxFrame` should be
prepared by OpenThread core, we expect no error and therefore assert
if parsing fails.
---
 src/core/mac/sub_mac.cpp | 46 +++++++++++++++++++++++++++++-----------
 src/core/mac/sub_mac.hpp |  1 +
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp
index e6383b133..0e76ef201 100644
--- a/src/core/mac/sub_mac.cpp
+++ b/src/core/mac/sub_mac.cpp
@@ -475,18 +475,7 @@ void SubMac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, otError aEr
         OT_UNREACHABLE_CODE(ExitNow());
     }
 
-    if (!ShouldHandleTransmitSecurity() && aFrame.GetSecurityEnabled())
-    {
-        uint8_t  keyIdMode;
-        uint32_t frameCounter = 0;
-
-        IgnoreError(aFrame.GetKeyIdMode(keyIdMode));
-        if (keyIdMode == Frame::kKeyIdMode1)
-        {
-            OT_ASSERT(aFrame.GetFrameCounter(frameCounter) == OT_ERROR_NONE);
-            UpdateFrameCounter(frameCounter);
-        }
-    }
+    UpdateFrameCounterOnTxDone(aFrame);
 
     // Determine whether a CSMA retry is required.
 
@@ -522,6 +511,39 @@ exit:
     return;
 }
 
+void SubMac::UpdateFrameCounterOnTxDone(const TxFrame &aFrame)
+{
+    uint8_t  keyIdMode;
+    uint32_t frameCounter;
+    bool     allowError = false;
+
+    OT_UNUSED_VARIABLE(allowError);
+
+    VerifyOrExit(!ShouldHandleTransmitSecurity() && aFrame.GetSecurityEnabled(), OT_NOOP);
+
+    // In an FTD/MTD build, if/when link-raw is enabled, the `TxFrame`
+    // is prepared and given by user and may not necessarily follow 15.4
+    // frame format (link raw can be used with vendor-specific format),
+    // so we allow failure when parsing the frame (i.e., do not assert
+    // on an error). In other cases (in an RCP build or in an FTD/MTD
+    // build without link-raw) since the `TxFrame` should be prepared by
+    // OpenThread core, we expect no error and therefore assert if
+    // parsing fails.
+
+#if OPENTHREAD_CONFIG_LINK_RAW_ENABLE
+    allowError = Get<LinkRaw>().IsEnabled();
+#endif
+
+    VerifyOrExit(aFrame.GetKeyIdMode(keyIdMode) == OT_ERROR_NONE, OT_ASSERT(allowError));
+    VerifyOrExit(keyIdMode == Frame::kKeyIdMode1, OT_NOOP);
+
+    VerifyOrExit(aFrame.GetFrameCounter(frameCounter) == OT_ERROR_NONE, OT_ASSERT(allowError));
+    UpdateFrameCounter(frameCounter);
+
+exit:
+    return;
+}
+
 int8_t SubMac::GetRssi(void) const
 {
     return Get<Radio>().GetRssi();
diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp
index 23bcf5021..7c08fcdf4 100644
--- a/src/core/mac/sub_mac.hpp
+++ b/src/core/mac/sub_mac.hpp
@@ -545,6 +545,7 @@ private:
     void HandleReceiveDone(RxFrame *aFrame, otError aError);
     void HandleTransmitStarted(TxFrame &aFrame);
     void HandleTransmitDone(TxFrame &aTxFrame, RxFrame *aAckFrame, otError aError);
+    void UpdateFrameCounterOnTxDone(const TxFrame &aFrame);
     void HandleEnergyScanDone(int8_t aMaxRssi);
 
     static void HandleTimer(Timer &aTimer);