Skip to content

Commit

Permalink
[Infineon] Fix CYW30739 KVS to pass the storage API audit. (project-c…
Browse files Browse the repository at this point in the history 
…hip#21995)

* Support key length upto PersistentStorageDelegate::kKeyLengthMax.
* _Get and _Put methods can handle zero-size data correctly.
  • Loading branch information
@hsusid @isiu-apple
hsusid authored and isiu-apple committed Sep 16, 2022
1 parent 4661bb0 commit bab3a45
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 24 deletions.
56 changes: 35 additions & 21 deletions src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<KeyConfigIdEntry>(configID, key, keyLength);
KeyConfigIdEntry * entry = Platform::New<KeyConfigIdEntry>(configID, keyStorage);
VerifyOrExit(entry != nullptr, err = CHIP_ERROR_NO_MEMORY);

slist_add_tail(entry, &mKeyConfigIdList);
Expand All @@ -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);
Expand All @@ -94,27 +103,33 @@ 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;
}

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);

entry = AllocateEntry(key, keyLength);
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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
Expand All @@ -175,7 +189,7 @@ KeyValueStoreManagerImpl::KeyConfigIdEntry * KeyValueStoreManagerImpl::AllocateE
ReturnErrorCodeIf(newEntry != nullptr, newEntry);
ReturnErrorCodeIf(!freeConfigID.HasValue(), nullptr);

newEntry = Platform::New<KeyConfigIdEntry>(freeConfigID.Value(), key, keyLength);
newEntry = Platform::New<KeyConfigIdEntry>(freeConfigID.Value(), KeyStorage(key, keyLength));
ReturnErrorCodeIf(newEntry == nullptr, nullptr);

KeyConfigIdEntry * entry = static_cast<KeyConfigIdEntry *>(slist_tail(&mKeyConfigIdList));
Expand Down Expand Up @@ -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))
Expand Down
18 changes: 15 additions & 3 deletions src/platform/Infineon/CYW30739/KeyValueStoreManagerImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#pragma once

#include <lib/core/CHIPPersistentStorageDelegate.h>
#include <platform/CHIPDeviceLayer.h>
#include <slist.h>

Expand Down Expand Up @@ -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);
Expand All @@ -71,9 +81,11 @@ class KeyValueStoreManagerImpl final : public KeyValueStoreManager
}
constexpr KeyConfigIdEntry * Next() const { return static_cast<KeyConfigIdEntry *>(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);
Expand Down

0 comments on commit bab3a45

Please sign in to comment.