[OpenThread] Optimize memory usage of DNS client (#8614)

In the worst-case scenario, the DNS client must correctly
handle a commissionable node service which contains
a TXT entry with key "PI" and a pairing instruction up to
128 characters. Change the method of allocating memory
for the resolved/discovered services to allow such long
TXT entries yet not to consume too much memory on stack.

src/lib/mdns/Advertiser.h

@@ -107,6 +107,7 @@ class OperationalAdvertisingParameters : public BaseAdvertisingParams<Operationa
     static constexpr uint8_t kTxtMaxNumber     = 2;
     static constexpr uint8_t kTxtMaxKeySize    = MaxStringLength("CRI", "CRA"); // possible keys
     static constexpr uint8_t kTxtMaxValueSize  = std::max({ kTxtRetryIntervalIdleMaxLength, kTxtRetryIntervalActiveMaxLength });
+    static constexpr size_t kTxtTotalKeySize   = TotalStringLength("CRI", "CRA"); // possible keys
     static constexpr size_t kTxtTotalValueSize = kTxtRetryIntervalIdleMaxLength + kTxtRetryIntervalActiveMaxLength;
 
     OperationalAdvertisingParameters & SetPeerId(const PeerId & peerId)
@@ -143,6 +144,7 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
         std::max({ kKeyDiscriminatorMaxLength, kKeyVendorProductMaxLength, kKeyAdditionalPairingMaxLength,
                    kKeyCommissioningModeMaxLength, kKeyDeviceTypeMaxLength, kKeyDeviceNameMaxLength, kKeyRotatingIdMaxLength,
                    kKeyPairingInstructionMaxLength, kKeyPairingHintMaxLength });
+    static constexpr size_t kTxtTotalKeySize   = TotalStringLength("D", "VP", "CM", "DT", "DN", "RI", "PI", "PH"); // possible keys
     static constexpr size_t kTxtTotalValueSize = kKeyDiscriminatorMaxLength + kKeyVendorProductMaxLength +
         kKeyAdditionalPairingMaxLength + kKeyCommissioningModeMaxLength + kKeyDeviceTypeMaxLength + kKeyDeviceNameMaxLength +
         kKeyRotatingIdMaxLength + kKeyPairingInstructionMaxLength + kKeyPairingHintMaxLength;

src/lib/support/SafeString.h

@@ -51,4 +51,27 @@ constexpr size_t MaxStringLength(const char (&)[FirstLength], RestOfTypes &&...
     return max(FirstLength - 1, MaxStringLength(std::forward<RestOfTypes>(aArgs)...));
 }
 
+/**
+ * A function that determines, at compile time, the total length (not
+ * counting the null terminator) of a list of literal C strings. Suitable for
+ * determining sizes of buffers that might need to hold all of the given
+ * strings.
+ *
+ * Do NOT pass things that are not string literals to this function.
+ *
+ * Use like:
+ *   constexpr size_t totalLen = TotalStringLength("abc", "defhij", "something");
+ */
+constexpr size_t TotalStringLength()
+{
+    return 0;
+}
+
+template <size_t FirstLength, typename... RestOfTypes>
+constexpr size_t TotalStringLength(const char (&)[FirstLength], RestOfTypes &&... aArgs)
+{
+    // Subtract 1 because we are not counting the null-terminator.
+    return FirstLength - 1 + TotalStringLength(std::forward<RestOfTypes>(aArgs)...);
+}
+
 } // namespace chip

src/lib/support/tests/TestSafeString.cpp

@@ -41,11 +41,18 @@ static void TestMaxStringLength(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, MaxStringLength("") == 0);
 }
 
+static void TestTotalStringLength(nlTestSuite * inSuite, void * inContext)
+{
+    NL_TEST_ASSERT(inSuite, TotalStringLength("") == 0);
+    NL_TEST_ASSERT(inSuite, TotalStringLength("a") == 1);
+    NL_TEST_ASSERT(inSuite, TotalStringLength("def", "bc", "a") == 6);
+}
+
 #define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn)
 /**
  *   Test Suite. It lists all the test functions.
  */
-static const nlTest sTests[] = { NL_TEST_DEF_FN(TestMaxStringLength), NL_TEST_SENTINEL() };
+static const nlTest sTests[] = { NL_TEST_DEF_FN(TestMaxStringLength), NL_TEST_DEF_FN(TestTotalStringLength), NL_TEST_SENTINEL() };
 
 int TestSafeString(void)
 {

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -1325,22 +1325,22 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
     otDnsInitTxtEntryIterator(&iterator, serviceInfo.mTxtData, serviceInfo.mTxtDataSize);
 
     otDnsTxtEntry txtEntry;
+    FixedBufferAllocator alloc(serviceTxtEntries.mBuffer);
 
     uint8_t entryIndex = 0;
     while ((otDnsGetNextTxtEntry(&iterator, &txtEntry) == OT_ERROR_NONE) && entryIndex < kMaxDnsServiceTxtEntriesNumber)
     {
-        if (txtEntry.mKey && strlen(txtEntry.mKey) <= kMaxDnsServiceTxtKeySize && txtEntry.mValue &&
-            txtEntry.mValueLength <= kMaxDnsServiceTxtValueSize)
-        {
-            strcpy(serviceTxtEntries.mTxtKeyBuffers[entryIndex], txtEntry.mKey);
-            serviceTxtEntries.mTxtEntries[entryIndex].mKey      = serviceTxtEntries.mTxtKeyBuffers[entryIndex];
-            serviceTxtEntries.mTxtEntries[entryIndex].mDataSize = txtEntry.mValueLength;
-            memcpy(serviceTxtEntries.mTxtValueBuffers[entryIndex], txtEntry.mValue, txtEntry.mValueLength);
-            serviceTxtEntries.mTxtEntries[entryIndex].mData = serviceTxtEntries.mTxtValueBuffers[entryIndex];
-            entryIndex++;
-        }
+        if (txtEntry.mKey == nullptr || txtEntry.mValue == nullptr)
+            continue;
+
+        serviceTxtEntries.mTxtEntries[entryIndex].mKey      = alloc.Clone(txtEntry.mKey);
+        serviceTxtEntries.mTxtEntries[entryIndex].mData     = alloc.Clone(txtEntry.mValue, txtEntry.mValueLength);
+        serviceTxtEntries.mTxtEntries[entryIndex].mDataSize = txtEntry.mValueLength;
+        entryIndex++;
     }
 
+    ReturnErrorCodeIf(alloc.AnyAllocFailed(), CHIP_ERROR_BUFFER_TOO_SMALL);
+
     mdnsService.mTextEntries   = serviceTxtEntries.mTxtEntries;
     mdnsService.mTextEntrySize = entryIndex;
 
@@ -1357,9 +1357,9 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsBrowseResult(otEr
     char type[chip::Mdns::kMdnsTypeAndProtocolMaxSize + SrpClient::kMaxDomainNameSize + 3];
     // hostname buffer size is kMdnsHostNameMaxSize + . + kMaxDomainNameSize + . + termination character
     char hostname[chip::Mdns::kMdnsHostNameMaxSize + SrpClient::kMaxDomainNameSize + 3];
-
-    uint8_t txtBuffer[kMaxDnsServiceTxtEntriesNumber *
-                      (kMaxDnsServiceTxtKeySize + 1 + kMaxDnsServiceTxtValueSize + sizeof(chip::Mdns::TextEntry))];
+    // secure space for the raw TXT data in the worst-case scenario relevant for Matter:
+    // each entry consists of txt_entry_size (1B) + txt_entry_key + "=" + txt_entry_data
+    uint8_t txtBuffer[kMaxDnsServiceTxtEntriesNumber + kTotalDnsServiceTxtBufferSize];
     otDnsServiceInfo serviceInfo;
     uint16_t index = 0;
     bool wasAnythingBrowsed;
@@ -1448,8 +1448,9 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsResolveResult(otE
     char type[chip::Mdns::kMdnsTypeAndProtocolMaxSize + SrpClient::kMaxDomainNameSize + 3];
     // hostname buffer size is kMdnsHostNameMaxSize + . + kMaxDomainNameSize + . + termination character
     char hostname[chip::Mdns::kMdnsHostNameMaxSize + SrpClient::kMaxDomainNameSize + 3];
-    uint8_t txtBuffer[kMaxDnsServiceTxtEntriesNumber *
-                      (kMaxDnsServiceTxtKeySize + 1 + kMaxDnsServiceTxtValueSize + sizeof(chip::Mdns::TextEntry))];
+    // secure space for the raw TXT data in the worst-case scenario relevant for Matter:
+    // each entry consists of txt_entry_size (1B) + txt_entry_key + "=" + txt_entry_data
+    uint8_t txtBuffer[kMaxDnsServiceTxtEntriesNumber + kTotalDnsServiceTxtBufferSize];
     otDnsServiceInfo serviceInfo;
 
     if (ThreadStackMgrImpl().mDnsResolveCallback == nullptr)

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.h

@@ -182,25 +182,28 @@ class GenericThreadStackManagerImpl_OpenThread
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
     // Thread supports both operational and commissionable discovery, so buffers sizes must be worst case.
-    static constexpr uint8_t kMaxDnsServiceTxtEntriesNumber = Mdns::CommissionAdvertisingParameters::kTxtMaxNumber;
-    static constexpr uint8_t kMaxDnsServiceTxtValueSize     = Mdns::CommissionAdvertisingParameters::kTxtMaxValueSize;
-    // TODO: Switch to max(commissionable TXT key size, operational TXT key size) after optimizing memory usage
-    static constexpr uint8_t kMaxDnsServiceTxtKeySize = Mdns::OperationalAdvertisingParameters::kTxtMaxKeySize;
+    static constexpr uint8_t kMaxDnsServiceTxtEntriesNumber =
+        std::max(Mdns::CommissionAdvertisingParameters::kTxtMaxNumber, Mdns::OperationalAdvertisingParameters::kTxtMaxNumber);
+    static constexpr size_t kTotalDnsServiceTxtValueSize = std::max(Mdns::CommissionAdvertisingParameters::kTxtTotalValueSize,
+                                                                    Mdns::OperationalAdvertisingParameters::kTxtTotalValueSize);
+    static constexpr size_t kTotalDnsServiceTxtKeySize =
+        std::max(Mdns::CommissionAdvertisingParameters::kTxtTotalKeySize, Mdns::OperationalAdvertisingParameters::kTxtTotalKeySize);
 #else
     // Thread only supports operational discovery.
     static constexpr uint8_t kMaxDnsServiceTxtEntriesNumber = Mdns::OperationalAdvertisingParameters::kTxtMaxNumber;
-    static constexpr uint8_t kMaxDnsServiceTxtValueSize     = Mdns::OperationalAdvertisingParameters::kTxtMaxValueSize;
-    static constexpr uint8_t kMaxDnsServiceTxtKeySize       = Mdns::OperationalAdvertisingParameters::kTxtMaxKeySize;
+    static constexpr size_t kTotalDnsServiceTxtValueSize    = Mdns::OperationalAdvertisingParameters::kTxtTotalValueSize;
+    static constexpr size_t kTotalDnsServiceTxtKeySize      = Mdns::OperationalAdvertisingParameters::kTxtTotalKeySize;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
+    static constexpr size_t kTotalDnsServiceTxtBufferSize =
+        kTotalDnsServiceTxtKeySize + kMaxDnsServiceTxtEntriesNumber + kTotalDnsServiceTxtValueSize;
 
     DnsBrowseCallback mDnsBrowseCallback;
     DnsResolveCallback mDnsResolveCallback;
 
     struct DnsServiceTxtEntries
     {
+        uint8_t mBuffer[kTotalDnsServiceTxtBufferSize];
         chip::Mdns::TextEntry mTxtEntries[kMaxDnsServiceTxtEntriesNumber];
-        uint8_t mTxtValueBuffers[kMaxDnsServiceTxtEntriesNumber][kMaxDnsServiceTxtValueSize];
-        char mTxtKeyBuffers[kMaxDnsServiceTxtEntriesNumber][kMaxDnsServiceTxtKeySize + 1];
     };
 
     struct DnsResult