[NXP] Some fixes for k32w platforms (#32684)

* [NXP][k32w0] Update KVS logs to specify storage name Signed-off-by: marius-alex-tache <[email protected]> * [NXP][k32w] Sanitize temporary buffer * Clear buffer that just handled the private key. The other variables (P256Keypair, P256SerializedKeypair) are automatically cleared when destroyed. Signed-off-by: Andrei Menzopol <[email protected]> * [NXP][k32w] Update contact sensor zap files * update Basic Information cluster revision * update ICDM cluster revision Signed-off-by: Doru Gucea <[email protected]> * [NXP][k32w1] Fix ECDSA signature validation Signed-off-by: Doru Gucea <[email protected]> * [NXP][k32w0] Add check for BLE connection when doing save on idle to improve commissioning behavior * [NXP][k32w0] Remove code that generates errors in debug builds for k32w0 --------- Signed-off-by: marius-alex-tache <[email protected]> Signed-off-by: Andrei Menzopol <[email protected]> Signed-off-by: Doru Gucea <[email protected]> Co-authored-by: Andrei Menzopol <[email protected]> Co-authored-by: Doru Gucea <[email protected]> Co-authored-by: Mihai Ignat <[email protected]>
project-chip · Apr 29, 2024 · 1772768 · 1772768
1 parent d1f6852
commit 1772768
Show file tree

Hide file tree

Showing 17 changed files with 101 additions and 79 deletions.
diff --git a/examples/contact-sensor-app/nxp/k32w/k32w0/main/AppTask.cpp b/examples/contact-sensor-app/nxp/k32w/k32w0/main/AppTask.cpp
@@ -979,3 +979,8 @@ extern "C" void OTAIdleActivities(void)
     OTA_TransactionResume();
 #endif
 }
+
+extern "C" bool AppHaveBLEConnections(void)
+{
+    return sHaveBLEConnections;
+}
diff --git a/examples/contact-sensor-app/nxp/zap-lit/contact-sensor-app.matter b/examples/contact-sensor-app/nxp/zap-lit/contact-sensor-app.matter
@@ -1453,7 +1453,7 @@ endpoint 0 {
     callback attribute eventList;
     callback attribute attributeList;
     ram      attribute featureMap default = 0;
-    ram      attribute clusterRevision default = 2;
+    ram      attribute clusterRevision default = 3;
   }
 
   server cluster OtaSoftwareUpdateRequestor {

diff --git a/examples/contact-sensor-app/nxp/zap-lit/contact-sensor-app.zap b/examples/contact-sensor-app/nxp/zap-lit/contact-sensor-app.zap
@@ -200,7 +200,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -264,7 +264,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -280,7 +280,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -296,7 +296,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -690,7 +690,7 @@
               "storageOption": "RAM",
               "singleton": 1,
               "bounded": 0,
-              "defaultValue": "2",
+              "defaultValue": "3",
               "reportable": 1,
               "minInterval": 0,
               "maxInterval": 65344,
@@ -2945,7 +2945,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -2961,7 +2961,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -2977,7 +2977,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -2993,7 +2993,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,

diff --git a/examples/contact-sensor-app/nxp/zap-sit/contact-sensor-app.matter b/examples/contact-sensor-app/nxp/zap-sit/contact-sensor-app.matter
@@ -1453,7 +1453,7 @@ endpoint 0 {
     callback attribute eventList;
     callback attribute attributeList;
     ram      attribute featureMap default = 0;
-    ram      attribute clusterRevision default = 2;
+    ram      attribute clusterRevision default = 3;
   }
 
   server cluster OtaSoftwareUpdateRequestor {
@@ -1692,7 +1692,7 @@ endpoint 0 {
     callback attribute eventList;
     callback attribute attributeList;
     ram      attribute featureMap default = 0x0000;
-    ram      attribute clusterRevision default = 1;
+    ram      attribute clusterRevision default = 2;
   }
 }
 endpoint 1 {

diff --git a/examples/contact-sensor-app/nxp/zap-sit/contact-sensor-app.zap b/examples/contact-sensor-app/nxp/zap-sit/contact-sensor-app.zap
@@ -200,7 +200,7 @@
               "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -690,7 +690,7 @@
               "storageOption": "RAM",
               "singleton": 1,
               "bounded": 0,
-              "defaultValue": "2",
+              "defaultValue": "3",
               "reportable": 1,
               "minInterval": 0,
               "maxInterval": 65344,
@@ -3679,7 +3679,7 @@
               "storageOption": "RAM",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "1",
+              "defaultValue": "2",
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,

diff --git a/examples/lighting-app/nxp/k32w/k32w0/main/AppTask.cpp b/examples/lighting-app/nxp/k32w/k32w0/main/AppTask.cpp
@@ -987,3 +987,8 @@ extern "C" void OTAIdleActivities(void)
     OTA_TransactionResume();
 #endif
 }
+
+extern "C" bool AppHaveBLEConnections(void)
+{
+    return sHaveBLEConnections;
+}
diff --git a/examples/platform/nxp/k32w/k32w0/app/support/FreeRtosHooks.c b/examples/platform/nxp/k32w/k32w0/app/support/FreeRtosHooks.c
@@ -252,10 +252,17 @@ static void BOARD_ActionOnIdle(void)
 }
 
 extern void OTAIdleActivities(void);
+extern bool AppHaveBLEConnections(void);
 
 void vApplicationIdleHook(void)
 {
-    FS_vIdleTask(PDM_MAX_WRITES_INFINITE);
+#if PDM_SAVE_IDLE
+    /* While in BLE connection during commissioning, PDM saves should be paused */
+    if (!AppHaveBLEConnections())
+    {
+        FS_vIdleTask(PDM_MAX_WRITES_INFINITE);
+    }
+#endif
 
     OTAIdleActivities();
 

diff --git a/src/platform/nxp/k32w/common/OTAImageProcessorImpl.cpp b/src/platform/nxp/k32w/common/OTAImageProcessorImpl.cpp
@@ -407,7 +407,6 @@ CHIP_ERROR OTAImageProcessorImpl::ReleaseBlock()
 
 void OTAImageProcessorImpl::FetchNextData(uint32_t context)
 {
-    CHIP_ERROR error      = CHIP_NO_ERROR;
     auto * imageProcessor = &OTAImageProcessorImpl::GetDefaultInstance();
     SystemLayer().ScheduleLambda([imageProcessor] {
         if (imageProcessor->mDownloader)

diff --git a/src/platform/nxp/k32w/k32w0/ConfigurationManagerImpl.cpp b/src/platform/nxp/k32w/k32w0/ConfigurationManagerImpl.cpp
@@ -276,8 +276,6 @@ CHIP_ERROR ConfigurationManagerImpl::DetermineBootReason(uint8_t rebootCause)
 
 void ConfigurationManagerImpl::DoFactoryReset(intptr_t arg)
 {
-    CHIP_ERROR err;
-
     ChipLogProgress(DeviceLayer, "Performing factory reset");
 
     K32WConfig::FactoryResetConfig();

diff --git a/src/platform/nxp/k32w/k32w0/FactoryDataProviderImpl.cpp b/src/platform/nxp/k32w/k32w0/FactoryDataProviderImpl.cpp
@@ -55,31 +55,40 @@ CHIP_ERROR FactoryDataProviderImpl::Init()
 
 CHIP_ERROR FactoryDataProviderImpl::SignWithDacKey(const ByteSpan & messageToSign, MutableByteSpan & outSignBuffer)
 {
+    CHIP_ERROR error = CHIP_NO_ERROR;
     Crypto::P256ECDSASignature signature;
     Crypto::P256Keypair keypair;
     Crypto::P256SerializedKeypair serializedKeypair;
-
-    VerifyOrReturnError(!outSignBuffer.empty(), CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(!messageToSign.empty(), CHIP_ERROR_INVALID_ARGUMENT);
-    VerifyOrReturnError(outSignBuffer.size() >= signature.Capacity(), CHIP_ERROR_BUFFER_TOO_SMALL);
-
-    /* Get private key of DAC certificate from reserved section */
     uint8_t keyBuf[Crypto::kP256_PrivateKey_Length];
     MutableByteSpan dacPrivateKeySpan(keyBuf);
     uint16_t keySize = 0;
-    ReturnErrorOnFailure(SearchForId(FactoryDataId::kDacPrivateKeyId, dacPrivateKeySpan.data(), dacPrivateKeySpan.size(), keySize));
+
+    VerifyOrExit(!outSignBuffer.empty(), error = CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrExit(!messageToSign.empty(), error = CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrExit(outSignBuffer.size() >= signature.Capacity(), error = CHIP_ERROR_BUFFER_TOO_SMALL);
+
+    /* Get private key of DAC certificate from reserved section */
+    error = SearchForId(FactoryDataId::kDacPrivateKeyId, dacPrivateKeySpan.data(), dacPrivateKeySpan.size(), keySize);
+    SuccessOrExit(error);
     dacPrivateKeySpan.reduce_size(keySize);
 
     /* Only the private key is used when signing */
-    ReturnErrorOnFailure(serializedKeypair.SetLength(Crypto::kP256_PublicKey_Length + dacPrivateKeySpan.size()));
+    error = serializedKeypair.SetLength(Crypto::kP256_PublicKey_Length + dacPrivateKeySpan.size());
+    SuccessOrExit(error);
     memcpy(serializedKeypair.Bytes() + Crypto::kP256_PublicKey_Length, dacPrivateKeySpan.data(), dacPrivateKeySpan.size());
 
-    ReturnErrorOnFailure(keypair.Deserialize(serializedKeypair));
-    ReturnErrorOnFailure(keypair.ECDSA_sign_msg(messageToSign.data(), messageToSign.size(), signature));
+    error = keypair.Deserialize(serializedKeypair);
+    SuccessOrExit(error);
 
-    // TODO: sanitize temporary buffers used to store the private key, so it doesn't leak on the stack?
+    error = keypair.ECDSA_sign_msg(messageToSign.data(), messageToSign.size(), signature);
+    SuccessOrExit(error);
 
-    return CopySpanToMutableSpan(ByteSpan{ signature.ConstBytes(), signature.Length() }, outSignBuffer);
+    error = CopySpanToMutableSpan(ByteSpan{ signature.ConstBytes(), signature.Length() }, outSignBuffer);
+
+exit:
+    /* Sanitize temporary buffer */
+    memset(keyBuf, 0, Crypto::kP256_PrivateKey_Length);
+    return error;
 }
 
 #if CONFIG_CHIP_K32W0_OTA_FACTORY_DATA_PROCESSOR

diff --git a/src/platform/nxp/k32w/k32w0/K32W0Config.cpp b/src/platform/nxp/k32w/k32w0/K32W0Config.cpp
@@ -36,9 +36,9 @@ namespace chip {
 namespace DeviceLayer {
 namespace Internal {
 
-RamStorage K32WConfig::sFactoryStorage{ kNvmId_Factory };
-RamStorage K32WConfig::sConfigStorage{ kNvmId_Config };
-RamStorage K32WConfig::sCounterStorage{ kNvmId_Counter };
+RamStorage K32WConfig::sFactoryStorage{ kNvmId_Factory, "Factory" };
+RamStorage K32WConfig::sConfigStorage{ kNvmId_Config, "Config" };
+RamStorage K32WConfig::sCounterStorage{ kNvmId_Counter, "Counter" };
 
 const K32WConfig::Key K32WConfig::kConfigKey_SerialNum{ &K32WConfig::sFactoryStorage, kKeyId_Factory, 0x00 };
 const K32WConfig::Key K32WConfig::kConfigKey_MfrDeviceId{ &K32WConfig::sFactoryStorage, kKeyId_Factory, 0x01 };

diff --git a/src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp b/src/platform/nxp/k32w/k32w0/KeyValueStoreManagerImpl.cpp
@@ -37,10 +37,10 @@ namespace PersistedStorage {
 constexpr size_t kMaxNumberOfKeys  = 200;
 constexpr size_t kMaxKeyValueBytes = 255;
 
-Internal::RamStorage KeyValueStoreManagerImpl::sKeysStorage         = { kNvmId_KvsKeys };
-Internal::RamStorage KeyValueStoreManagerImpl::sValuesStorage       = { kNvmId_KvsValues };
-Internal::RamStorage KeyValueStoreManagerImpl::sSubscriptionStorage = { kNvmId_KvsSubscription };
-Internal::RamStorage KeyValueStoreManagerImpl::sGroupsStorage       = { kNvmId_KvsGroups };
+Internal::RamStorage KeyValueStoreManagerImpl::sKeysStorage         = { kNvmId_KvsKeys, "Keys" };
+Internal::RamStorage KeyValueStoreManagerImpl::sValuesStorage       = { kNvmId_KvsValues, "Values" };
+Internal::RamStorage KeyValueStoreManagerImpl::sSubscriptionStorage = { kNvmId_KvsSubscription, "Subscriptions" };
+Internal::RamStorage KeyValueStoreManagerImpl::sGroupsStorage       = { kNvmId_KvsGroups, "Groups" };
 
 KeyValueStoreManagerImpl KeyValueStoreManagerImpl::sInstance;
 
@@ -169,13 +169,13 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
     {
         /* Use kKeyId_KvsValues as base key id for all keys. */
         pdmInternalId = Internal::RamStorageKey::GetInternalId(kKeyId_KvsValues, keyId);
-        ChipLogProgress(DeviceLayer, "KVS, get the value of Matter key [%s] with PDM id: %i", key, pdmInternalId);
+        ChipLogProgress(DeviceLayer, "KVS val: get [%s][%i][%s]", key, pdmInternalId, GetValStorage(key)->GetName());
         err              = GetValStorage(key)->Read(pdmInternalId, 0, (uint8_t *) value, &valueSize);
         *read_bytes_size = valueSize;
     }
     else
     {
-        ChipLogProgress(DeviceLayer, "KVS, Matter key [%s] not found in persistent storage.", key);
+        ChipLogProgress(DeviceLayer, "KVS key [%s] not found in persistent storage.", key);
     }
 
 exit:
@@ -204,30 +204,26 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value,
 
     /* Use kKeyId_KvsValues as base key id for all keys. */
     pdmInternalId = Internal::RamStorageKey::GetInternalId(kKeyId_KvsValues, keyId);
-    ChipLogProgress(DeviceLayer, "KVS, save in flash the value of the Matter key [%s] with PDM id: %i", key, pdmInternalId);
-
+    ChipLogProgress(DeviceLayer, "KVS val: set [%s][%i][%s]", key, pdmInternalId, GetValStorage(key)->GetName());
     err = GetValStorage(key)->Write(pdmInternalId, (uint8_t *) value, value_size);
     /* save the 'key' in flash such that it can be retrieved later on */
     if (err == CHIP_NO_ERROR)
     {
         if (putKey)
         {
             pdmInternalId = Internal::RamStorageKey::GetInternalId(kKeyId_KvsKeys, keyId);
-            ChipLogProgress(DeviceLayer, "KVS, save in flash the Matter key [%s] with PDM id: %i and length %d", key, pdmInternalId,
-                            strlen(key) + 1);
+            ChipLogProgress(DeviceLayer, "KVS key: set [%s][%i][%s]", key, pdmInternalId, GetKeyStorage(key)->GetName());
 
             err = GetKeyStorage(key)->Write(pdmInternalId, (uint8_t *) key, strlen(key) + 1);
             if (err != CHIP_NO_ERROR)
             {
-                ChipLogProgress(DeviceLayer, "KVS, Error while saving in flash the Matter key [%s] with PDM id: %i", key,
-                                pdmInternalId);
+                ChipLogProgress(DeviceLayer, "KVS key: error when setting [%s][%i]", key, pdmInternalId);
             }
         }
     }
     else
     {
-        ChipLogProgress(DeviceLayer, "KVS, Error while saving in flash the value of the Matter key [%s] with PDM id: %i", key,
-                        pdmInternalId);
+        ChipLogProgress(DeviceLayer, "KVS val: error when setting [%s][%i]", key, pdmInternalId);
     }
 
 exit:
@@ -249,29 +245,25 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key)
         // entry exists so we can remove it
         pdmInternalId = Internal::RamStorageKey::GetInternalId(kKeyId_KvsKeys, keyId);
 
-        ChipLogProgress(DeviceLayer, "KVS, delete from flash the Matter key [%s] with PDM id: %i", key, pdmInternalId);
+        ChipLogProgress(DeviceLayer, "KVS key: del [%s][%i][%s]", key, pdmInternalId, GetKeyStorage(key)->GetName());
         err = GetKeyStorage(key)->Delete(pdmInternalId, -1);
 
         /* also delete the 'key string' from flash */
         if (err == CHIP_NO_ERROR)
         {
             /* Use kKeyId_KvsValues as base key id for all keys. */
             pdmInternalId = Internal::RamStorageKey::GetInternalId(kKeyId_KvsValues, keyId);
-            ChipLogProgress(DeviceLayer, "KVS, delete from flash the value of the Matter key [%s] with PDM id: %i", key,
-                            pdmInternalId);
+            ChipLogProgress(DeviceLayer, "KVS val: del [%s][%i][%s]", key, pdmInternalId, GetValStorage(key)->GetName());
 
             err = GetValStorage(key)->Delete(pdmInternalId, -1);
             if (err != CHIP_NO_ERROR)
             {
-                ChipLogProgress(DeviceLayer,
-                                "KVS, Error while deleting from flash the value of the Matter key [%s] with PDM id: %i", key,
-                                pdmInternalId);
+                ChipLogProgress(DeviceLayer, "KVS val: error when deleting [%s][%i]", key, pdmInternalId);
             }
         }
         else
         {
-            ChipLogProgress(DeviceLayer, "KVS, Error while deleting from flash the Matter key [%s] with PDM id: %i", key,
-                            pdmInternalId);
+            ChipLogProgress(DeviceLayer, "KVS key: error when deleting [%s][%i]", key, pdmInternalId);
         }
     }
 exit:

diff --git a/src/platform/nxp/k32w/k32w0/RamStorage.cpp b/src/platform/nxp/k32w/k32w0/RamStorage.cpp
@@ -70,8 +70,6 @@ CHIP_ERROR RamStorageKey::Delete()
 
 CHIP_ERROR RamStorage::Init(uint16_t aInitialSize, bool extendedSearch)
 {
-    CHIP_ERROR err;
-
     mBuffer         = getRamBuffer(mPdmId, aInitialSize, extendedSearch);
     mExtendedSearch = extendedSearch;
 
@@ -162,13 +160,14 @@ void RamStorage::OnFactoryReset()
     {
         while (PDM_bDoesDataExist(mPdmId + i, &length))
         {
-            ChipLogProgress(DeviceLayer, "Ram Storage: delete PDM id: %x", mPdmId + i);
+            ChipLogProgress(DeviceLayer, "Ram Storage: delete PDM id: 0x%x", mPdmId + i);
             PDM_vDeleteDataRecord(mPdmId + i);
             i++;
         }
     }
     else
     {
+        ChipLogProgress(DeviceLayer, "Ram Storage: delete PDM id: 0x%x", mPdmId);
         PDM_vDeleteDataRecord(mPdmId);
     }
     mutex_unlock(mBuffer);