From bab3a45364b4d6893a86c61f67daa8b3e2708369 Mon Sep 17 00:00:00 2001 From: Sid Hsu Date: Tue, 23 Aug 2022 01:54:37 +0800 Subject: [PATCH] [Infineon] Fix CYW30739 KVS to pass the storage API audit. (#21995) * Support key length upto PersistentStorageDelegate::kKeyLengthMax. * _Get and _Put methods can handle zero-size data correctly. --- .../CYW30739/KeyValueStoreManagerImpl.cpp | 56 ++++++++++++------- .../CYW30739/KeyValueStoreManagerImpl.h | 18 +++++- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.cpp b/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.cpp index 00e7a063614bb9..e61edab6ebfd18 100644 --- a/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.cpp +++ b/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.cpp @@ -44,15 +44,14 @@ CHIP_ERROR KeyValueStoreManagerImpl::Init(void) for (uint8_t configID = 0; configID < mMaxEntryCount; configID++) { - char key[CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH]; - memset(key, 0, sizeof(key)); - size_t keyLength; - err = CYW30739Config::ReadConfigValueStr(CYW30739ConfigKey(Config::kChipKvsKey_KeyBase, configID), key, sizeof(key), - keyLength); + KeyStorage keyStorage; + size_t keyStorageLength; + err = CYW30739Config::ReadConfigValueBin(CYW30739ConfigKey(Config::kChipKvsKey_KeyBase, configID), &keyStorage, + sizeof(keyStorage), keyStorageLength); if (err != CHIP_NO_ERROR) continue; - KeyConfigIdEntry * entry = Platform::New(configID, key, keyLength); + KeyConfigIdEntry * entry = Platform::New(configID, keyStorage); VerifyOrExit(entry != nullptr, err = CHIP_ERROR_NO_MEMORY); slist_add_tail(entry, &mKeyConfigIdList); @@ -75,14 +74,24 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t VerifyOrReturnError(offset_bytes == 0, CHIP_ERROR_NOT_IMPLEMENTED); - const size_t keyLength = strnlen(key, CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH); - VerifyOrExit(keyLength != 0 && keyLength <= CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH && + const size_t keyLength = strnlen(key, PersistentStorageDelegate::kKeyLengthMax); + VerifyOrExit(keyLength != 0 && keyLength <= PersistentStorageDelegate::kKeyLengthMax && value_size <= kMaxPersistedValueLengthSupported, err = CHIP_ERROR_INVALID_ARGUMENT); entry = FindEntry(key); VerifyOrExit(entry != nullptr, err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND); - VerifyOrExit(value_size != 0, err = CHIP_ERROR_BUFFER_TOO_SMALL); + + if (value_size == 0 || entry->GetValueSize() == 0) + { + if (read_bytes_size != nullptr) + *read_bytes_size = 0; + + if (value_size >= entry->GetValueSize()) + ExitNow(err = CHIP_NO_ERROR); + else + ExitNow(err = CHIP_ERROR_BUFFER_TOO_SMALL); + } size_t byte_count; err = CYW30739Config::ReadConfigValueBin(entry->GetValueConfigKey(), value, value_size, byte_count); @@ -94,6 +103,8 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t *read_bytes_size = byte_count; } + VerifyOrExit(value_size >= entry->GetValueSize(), err = CHIP_ERROR_BUFFER_TOO_SMALL); + exit: return err; } @@ -101,10 +112,10 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size) { CHIP_ERROR err = CHIP_NO_ERROR; - const KeyConfigIdEntry * entry; + KeyConfigIdEntry * entry; - const size_t keyLength = strnlen(key, CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH + 1); - VerifyOrExit(keyLength != 0 && keyLength <= CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH && + const size_t keyLength = strnlen(key, PersistentStorageDelegate::kKeyLengthMax + 1); + VerifyOrExit(keyLength != 0 && keyLength <= PersistentStorageDelegate::kKeyLengthMax && value_size <= kMaxPersistedValueLengthSupported, err = CHIP_ERROR_INVALID_ARGUMENT); @@ -112,9 +123,13 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, VerifyOrExit(entry != nullptr, ChipLogError(DeviceLayer, "%s AllocateEntry %s", __func__, ErrorStr(err)); err = CHIP_ERROR_NO_MEMORY); - SuccessOrExit(err = CYW30739Config::WriteConfigValueBin(entry->GetValueConfigKey(), value, value_size)); + if (value_size != 0) + { + SuccessOrExit(err = CYW30739Config::WriteConfigValueBin(entry->GetValueConfigKey(), value, value_size)); + } - SuccessOrExit(err = CYW30739Config::WriteConfigValueStr(entry->GetKeyConfigKey(), key, keyLength)); + entry->SetValueSize(value_size); + SuccessOrExit(err = CYW30739Config::WriteConfigValueBin(entry->GetKeyConfigKey(), &entry->mStorage, sizeof(entry->mStorage))); exit: return err; @@ -125,8 +140,8 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key) CHIP_ERROR err; KeyConfigIdEntry * entry; - const size_t keyLength = strnlen(key, CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH); - VerifyOrExit(keyLength != 0 && keyLength <= CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH, err = CHIP_ERROR_INVALID_ARGUMENT); + const size_t keyLength = strnlen(key, PersistentStorageDelegate::kKeyLengthMax); + VerifyOrExit(keyLength != 0 && keyLength <= PersistentStorageDelegate::kKeyLengthMax, err = CHIP_ERROR_INVALID_ARGUMENT); entry = FindEntry(key); VerifyOrExit(entry != nullptr, err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND); @@ -156,14 +171,13 @@ CHIP_ERROR KeyValueStoreManagerImpl::EraseAll(void) return CHIP_NO_ERROR; } -KeyValueStoreManagerImpl::KeyConfigIdEntry::KeyConfigIdEntry(uint8_t configID, const char * key, size_t keyLength) : - mConfigID(configID) +KeyValueStoreManagerImpl::KeyStorage::KeyStorage(const char * key, size_t keyLength) : mValueSize(0) { memset(mKey, 0, sizeof(mKey)); memcpy(mKey, key, keyLength); } -bool KeyValueStoreManagerImpl::KeyConfigIdEntry::IsMatchKey(const char * key) const +bool KeyValueStoreManagerImpl::KeyStorage::IsMatchKey(const char * key) const { return strncmp(mKey, key, sizeof(mKey)) == 0; } @@ -175,7 +189,7 @@ KeyValueStoreManagerImpl::KeyConfigIdEntry * KeyValueStoreManagerImpl::AllocateE ReturnErrorCodeIf(newEntry != nullptr, newEntry); ReturnErrorCodeIf(!freeConfigID.HasValue(), nullptr); - newEntry = Platform::New(freeConfigID.Value(), key, keyLength); + newEntry = Platform::New(freeConfigID.Value(), KeyStorage(key, keyLength)); ReturnErrorCodeIf(newEntry == nullptr, nullptr); KeyConfigIdEntry * entry = static_cast(slist_tail(&mKeyConfigIdList)); @@ -220,7 +234,7 @@ KeyValueStoreManagerImpl::KeyConfigIdEntry * KeyValueStoreManagerImpl::FindEntry { entry = entry->Next(); - if (entry->IsMatchKey(key)) + if (entry->mStorage.IsMatchKey(key)) return entry; if (freeConfigID != nullptr && !freeConfigID->HasValue() && entry != slist_tail(&mKeyConfigIdList)) diff --git a/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.h b/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.h index a719bc6c2af7ac..5938bd28a56c98 100644 --- a/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.h +++ b/src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.h @@ -23,6 +23,7 @@ #pragma once +#include #include #include @@ -56,11 +57,20 @@ class KeyValueStoreManagerImpl final : public KeyValueStoreManager static constexpr uint8_t mMaxEntryCount = 128; - struct KeyConfigIdEntry : public slist_node_t + struct KeyStorage { - KeyConfigIdEntry(uint8_t configID, const char * key, size_t keyLength); + KeyStorage(const char * key = nullptr, size_t keyLength = 0); bool IsMatchKey(const char * key) const; + + size_t mValueSize; + char mKey[PersistentStorageDelegate::kKeyLengthMax]; + }; + + struct KeyConfigIdEntry : public slist_node_t + { + KeyConfigIdEntry(uint8_t configID, const KeyStorage & keyStorage) : mConfigID(configID), mStorage(keyStorage) {} + constexpr Config::Key GetValueConfigKey() const { return Internal::CYW30739ConfigKey(Config::kChipKvsValue_KeyBase, mConfigID); @@ -71,9 +81,11 @@ class KeyValueStoreManagerImpl final : public KeyValueStoreManager } constexpr KeyConfigIdEntry * Next() const { return static_cast(next); } constexpr uint8_t NextConfigID() const { return mConfigID + 1; } + constexpr size_t GetValueSize() const { return mStorage.mValueSize; } + constexpr void SetValueSize(size_t valueSize) { mStorage.mValueSize = valueSize; } uint8_t mConfigID; - char mKey[CHIP_CONFIG_PERSISTED_STORAGE_MAX_KEY_LENGTH]; + KeyStorage mStorage; }; KeyConfigIdEntry * AllocateEntry(const char * key, size_t keyLength);