From 6f3c05cac33b679941a07ece7f370e128c2b71db Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 29 Nov 2023 18:03:35 +0000 Subject: [PATCH 1/9] Prevent TLVWrite from possibly performing buffer overrun when calling reserve --- src/lib/core/TLVBackingStore.h | 2 + src/lib/core/TLVWriter.cpp | 16 +++ src/lib/core/TLVWriter.h | 11 +- src/system/TLVPacketBufferBackingStore.h | 5 + .../tests/TestTLVPacketBufferBackingStore.cpp | 116 +++++++++++++++++- 5 files changed, 141 insertions(+), 9 deletions(-) diff --git a/src/lib/core/TLVBackingStore.h b/src/lib/core/TLVBackingStore.h index 6d3c989e116327..23291a0d4d64c0 100644 --- a/src/lib/core/TLVBackingStore.h +++ b/src/lib/core/TLVBackingStore.h @@ -155,6 +155,8 @@ class DLL_EXPORT TLVBackingStore * */ virtual CHIP_ERROR FinalizeBuffer(TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) = 0; + + virtual bool IsSafeToReserve() { return false; } }; } // namespace TLV diff --git a/src/lib/core/TLVWriter.cpp b/src/lib/core/TLVWriter.cpp index eb93e781524677..fb04ecd7f2d96f 100644 --- a/src/lib/core/TLVWriter.cpp +++ b/src/lib/core/TLVWriter.cpp @@ -97,6 +97,22 @@ CHIP_ERROR TLVWriter::Finalize() return err; } +CHIP_ERROR TLVWriter::ReserveBuffer(uint32_t aBufferSize) +{ + VerifyOrReturnError(mRemainingLen >= aBufferSize, CHIP_ERROR_NO_MEMORY); + + uint32_t spaceLeftInCurrentBuffer = mReservedSize + mRemainingLen; + bool canPossiblyAllocateAdditionalMemory = spaceLeftInCurrentBuffer < mMaxLen; + + if (mBackingStore && canPossiblyAllocateAdditionalMemory) + { + VerifyOrReturnError(mBackingStore->IsSafeToReserve(), CHIP_ERROR_INCORRECT_STATE); + } + mReservedSize += aBufferSize; + mRemainingLen -= aBufferSize; + return CHIP_NO_ERROR; +} + CHIP_ERROR TLVWriter::PutBoolean(Tag tag, bool v) { return WriteElementHead((v) ? TLVElementType::BooleanTrue : TLVElementType::BooleanFalse, tag, 0); diff --git a/src/lib/core/TLVWriter.h b/src/lib/core/TLVWriter.h index 59478f15e7123c..76b3d9d15d042c 100644 --- a/src/lib/core/TLVWriter.h +++ b/src/lib/core/TLVWriter.h @@ -128,14 +128,11 @@ class DLL_EXPORT TLVWriter * * @retval #CHIP_NO_ERROR Successfully reserved required buffer size. * @retval #CHIP_ERROR_NO_MEMORY The reserved buffer size cannot fits into the remaining buffer size. + * @retval #CHIP_ERROR_INCORRECT_STATE + * Uses TLVBackingStore and is in a state where it might allocate + * additional non-contigious memory, thus making it impossible to */ - CHIP_ERROR ReserveBuffer(uint32_t aBufferSize) - { - VerifyOrReturnError(mRemainingLen >= aBufferSize, CHIP_ERROR_NO_MEMORY); - mReservedSize += aBufferSize; - mRemainingLen -= aBufferSize; - return CHIP_NO_ERROR; - } + CHIP_ERROR ReserveBuffer(uint32_t aBufferSize); /** * Release previously reserved buffer. diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index 81afe973ad3c33..bb7c91097824b0 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -80,6 +80,11 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR OnInit(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR GetNewBuffer(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; + virtual bool IsSafeToReserve() override + { + // It is safe to reserve if there is not chained buffers. + return !mUseChainedBuffers; + } protected: chip::System::PacketBufferHandle mHeadBuffer; diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index fccced4cfbb4f4..bcd3672435c718 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -35,6 +35,18 @@ using namespace ::chip; namespace { +void WriteUntilRemainingLessThan(nlTestSuite * inSuite, PacketBufferTLVWriter & writer, const uint32_t & remainingSize) +{ + CHIP_ERROR error; + uint32_t lengthRemaining = writer.GetRemainingFreeLength(); + while (lengthRemaining >= remainingSize) + { + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + lengthRemaining = writer.GetRemainingFreeLength(); + } +} + class TLVPacketBufferBackingStoreTest { public: @@ -43,6 +55,9 @@ class TLVPacketBufferBackingStoreTest static void BasicEncodeDecode(nlTestSuite * inSuite, void * inContext); static void MultiBufferEncode(nlTestSuite * inSuite, void * inContext); + static void NonChainedBufferCanReserve(nlTestSuite * inSuite, void * inContext); + static void TestWriterReserveUnreserveDoesNotOverflow(nlTestSuite * inSuite, void * inContext); + static void TestWriterReserve(nlTestSuite * inSuite, void * inContext); }; int TLVPacketBufferBackingStoreTest::TestSetup(void * inContext) @@ -238,14 +253,111 @@ void TLVPacketBufferBackingStoreTest::MultiBufferEncode(nlTestSuite * inSuite, v NL_TEST_ASSERT(inSuite, error == CHIP_END_OF_TLV); } +void TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve(nlTestSuite * inSuite, void * inContext) +{ + // Start with a too-small buffer. + uint32_t smallSize = 5; + uint32_t smallerSizeToReserver = smallSize - 1; + + auto buffer = PacketBufferHandle::New(smallSize, 0); + + PacketBufferTLVWriter writer; + writer.Init(std::move(buffer), /* useChainedBuffers = */ false); + + CHIP_ERROR error = writer.ReserveBuffer(smallerSizeToReserver); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); +} + +// This test previously was created to show that there was an overflow bug +void TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow(nlTestSuite * inSuite, void * inContext) +{ + // Start with a too-small buffer. + uint32_t smallSize = 100; + uint32_t smallerSizeToReserver = smallSize - 1; + + auto buffer = PacketBufferHandle::New(smallSize, 0); + + PacketBufferTLVWriter writer; + writer.Init(std::move(buffer), /* useChainedBuffers = */ true); + + CHIP_ERROR error = writer.ReserveBuffer(smallerSizeToReserver); + // Previously + if (error == CHIP_NO_ERROR) + { + uint32_t lengthRemaining = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(inSuite, lengthRemaining == 1); + // Lets try to overflow by getting next buffer in the chain, + // unreserving then writting until the end of the current buffer. + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + lengthRemaining = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(inSuite, lengthRemaining > smallerSizeToReserver); + + WriteUntilRemainingLessThan(inSuite, writer, 2); + + lengthRemaining = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(inSuite, lengthRemaining != 0); + NL_TEST_ASSERT(inSuite, lengthRemaining < smallerSizeToReserver); + + error = writer.UnreserveBuffer(smallerSizeToReserver); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + lengthRemaining = writer.GetRemainingFreeLength(); + NL_TEST_ASSERT(inSuite, lengthRemaining > smallerSizeToReserver); + + // This is where we get overflow. + WriteUntilRemainingLessThan(inSuite, writer, 2); + + // If we get here then the overflow condition we were expecting did not happen. If that is the case, + // either we have fixed reservation for chained buffers, or expected failure didn't hit on this + // platform. + // + // If there is a fix please add reservation for chained buffers, please make sure you account for + // what happens if TLVWriter::WriteData fails to get a new buffer but we are not at max size, do + // you actually have space for what was supposed to be reserved. + NL_TEST_ASSERT(inSuite, false); + } + + // We no longer allow non-contigous buffers to be reserved. + NL_TEST_ASSERT(inSuite, error == CHIP_ERROR_INCORRECT_STATE); +} + +void TLVPacketBufferBackingStoreTest::TestWriterReserve(nlTestSuite * inSuite, void * inContext) +{ + // Start with a too-small buffer. + uint32_t smallSize = 5; + uint32_t smallerSizeToReserver = smallSize - 1; + + auto buffer = PacketBufferHandle::New(smallSize, 0); + + PacketBufferTLVWriter writer; + writer.Init(std::move(buffer), /* useChainedBuffers = */ false); + + CHIP_ERROR error = writer.ReserveBuffer(smallerSizeToReserver); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_ERROR_NO_MEMORY); + + error = writer.UnreserveBuffer(smallerSizeToReserver); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + + error = writer.Put(TLV::AnonymousTag(), static_cast(7)); + NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); +} + /** * Test Suite. It lists all the test functions. */ // clang-format off const nlTest sTests[] = { - NL_TEST_DEF("BasicEncodeDecode", TLVPacketBufferBackingStoreTest::BasicEncodeDecode), - NL_TEST_DEF("MultiBufferEncode", TLVPacketBufferBackingStoreTest::MultiBufferEncode), + NL_TEST_DEF("BasicEncodeDecode", TLVPacketBufferBackingStoreTest::BasicEncodeDecode), + NL_TEST_DEF("MultiBufferEncode", TLVPacketBufferBackingStoreTest::MultiBufferEncode), + NL_TEST_DEF("NonChainedBufferCanReserve", TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve), + NL_TEST_DEF("TestWriterReserveUnreserveDoesNotOverflow", TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow), + NL_TEST_DEF("TestWriterReserve", TLVPacketBufferBackingStoreTest::TestWriterReserve), NL_TEST_SENTINEL() }; From c29bc8aaa3ba9062c8abdf20efc75ff42eab33fb Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 29 Nov 2023 18:22:21 +0000 Subject: [PATCH 2/9] Quick cleanup --- src/system/TLVPacketBufferBackingStore.h | 2 +- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index bb7c91097824b0..ea32ece84f1876 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -82,7 +82,7 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; virtual bool IsSafeToReserve() override { - // It is safe to reserve if there is not chained buffers. + // It is safe to reserve if there is no chained buffers. return !mUseChainedBuffers; } diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index bcd3672435c718..9837efc1923b76 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -268,7 +268,8 @@ void TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve(nlTestSuite * i NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); } -// This test previously was created to show that there was an overflow bug +// This test previously was created to show that there was an overflow bug, now this test mainly +// just checks that you cannot reserve this type of TLVBackingStorage buffer. void TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow(nlTestSuite * inSuite, void * inContext) { // Start with a too-small buffer. @@ -281,7 +282,6 @@ void TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow( writer.Init(std::move(buffer), /* useChainedBuffers = */ true); CHIP_ERROR error = writer.ReserveBuffer(smallerSizeToReserver); - // Previously if (error == CHIP_NO_ERROR) { uint32_t lengthRemaining = writer.GetRemainingFreeLength(); From 21e888d4ab7300585c819597455b6e30d9145dd2 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 29 Nov 2023 19:11:08 +0000 Subject: [PATCH 3/9] Address PR Comments --- src/lib/core/TLVWriter.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/core/TLVWriter.h b/src/lib/core/TLVWriter.h index 76b3d9d15d042c..9008d92b88d394 100644 --- a/src/lib/core/TLVWriter.h +++ b/src/lib/core/TLVWriter.h @@ -130,7 +130,8 @@ class DLL_EXPORT TLVWriter * @retval #CHIP_ERROR_NO_MEMORY The reserved buffer size cannot fits into the remaining buffer size. * @retval #CHIP_ERROR_INCORRECT_STATE * Uses TLVBackingStore and is in a state where it might allocate - * additional non-contigious memory, thus making it impossible to + * additional non-contigious memory, thus making it difficult/impossible + * to properly reserve space. */ CHIP_ERROR ReserveBuffer(uint32_t aBufferSize); From efa7c1011ee195ce9ea25e6cbc8763eb6c077f1e Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 30 Nov 2023 21:56:53 +0000 Subject: [PATCH 4/9] Address CI comments --- src/system/TLVPacketBufferBackingStore.h | 7 ++++++- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 8 +++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index ea32ece84f1876..10ffad58f76faf 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -82,7 +82,12 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; virtual bool IsSafeToReserve() override { - // It is safe to reserve if there is no chained buffers. + // When using chained buffers there is no guarantee what the size of new buffer will + // be when calling GetNewBuffer. This makes it very difficult for a caller to safely + // ensure there is always enough space at the end of new buffer for reservation, + // when going from one buffer to the next. + // For non-chained buffers, caller is given one chunk of conigous memory so it is + // possible to reserve with buffer currently in use by caller. return !mUseChainedBuffers; } diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index 9837efc1923b76..0f9ff3d7c2ef20 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -35,14 +35,12 @@ using namespace ::chip; namespace { -void WriteUntilRemainingLessThan(nlTestSuite * inSuite, PacketBufferTLVWriter & writer, const uint32_t & remainingSize) +void WriteUntilRemainingLessThan(nlTestSuite * inSuite, PacketBufferTLVWriter & writer, const uint32_t remainingSize) { - CHIP_ERROR error; uint32_t lengthRemaining = writer.GetRemainingFreeLength(); while (lengthRemaining >= remainingSize) { - error = writer.Put(TLV::AnonymousTag(), static_cast(7)); - NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, writer.Put(TLV::AnonymousTag(), static_cast(7)) == CHIP_NO_ERROR); lengthRemaining = writer.GetRemainingFreeLength(); } } @@ -259,7 +257,7 @@ void TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve(nlTestSuite * i uint32_t smallSize = 5; uint32_t smallerSizeToReserver = smallSize - 1; - auto buffer = PacketBufferHandle::New(smallSize, 0); + auto buffer = PacketBufferHandle::New(smallSize, /* reservedSize = */ 0); PacketBufferTLVWriter writer; writer.Init(std::move(buffer), /* useChainedBuffers = */ false); From b23c7375d6a68f726f9ff0d7b75bb55ded3623df Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 1 Dec 2023 14:15:19 +0000 Subject: [PATCH 5/9] Address CI comments --- src/lib/core/TLVBackingStore.h | 2 +- src/lib/core/TLVWriter.cpp | 7 ++----- src/system/TLVPacketBufferBackingStore.h | 10 +++------- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 2 +- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/lib/core/TLVBackingStore.h b/src/lib/core/TLVBackingStore.h index 23291a0d4d64c0..1e232c24ed79f7 100644 --- a/src/lib/core/TLVBackingStore.h +++ b/src/lib/core/TLVBackingStore.h @@ -156,7 +156,7 @@ class DLL_EXPORT TLVBackingStore */ virtual CHIP_ERROR FinalizeBuffer(TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) = 0; - virtual bool IsSafeToReserve() { return false; } + virtual bool GetNewBufferWillAlwaysFail() { return false; } }; } // namespace TLV diff --git a/src/lib/core/TLVWriter.cpp b/src/lib/core/TLVWriter.cpp index fb04ecd7f2d96f..99e44f0753e73e 100644 --- a/src/lib/core/TLVWriter.cpp +++ b/src/lib/core/TLVWriter.cpp @@ -101,12 +101,9 @@ CHIP_ERROR TLVWriter::ReserveBuffer(uint32_t aBufferSize) { VerifyOrReturnError(mRemainingLen >= aBufferSize, CHIP_ERROR_NO_MEMORY); - uint32_t spaceLeftInCurrentBuffer = mReservedSize + mRemainingLen; - bool canPossiblyAllocateAdditionalMemory = spaceLeftInCurrentBuffer < mMaxLen; - - if (mBackingStore && canPossiblyAllocateAdditionalMemory) + if (mBackingStore) { - VerifyOrReturnError(mBackingStore->IsSafeToReserve(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(mBackingStore->GetNewBufferWillAlwaysFail(), CHIP_ERROR_INCORRECT_STATE); } mReservedSize += aBufferSize; mRemainingLen -= aBufferSize; diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index 10ffad58f76faf..6702cda259cfff 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -80,14 +80,10 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR OnInit(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR GetNewBuffer(chip::TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; - virtual bool IsSafeToReserve() override + virtual bool GetNewBufferWillAlwaysFail() override { - // When using chained buffers there is no guarantee what the size of new buffer will - // be when calling GetNewBuffer. This makes it very difficult for a caller to safely - // ensure there is always enough space at the end of new buffer for reservation, - // when going from one buffer to the next. - // For non-chained buffers, caller is given one chunk of conigous memory so it is - // possible to reserve with buffer currently in use by caller. + // For non-chained buffers, caller is given one chunk of contiguous memory all calls to + // GetNewBuffer will fail with CHIP_ERROR_NO_MEMORY. return !mUseChainedBuffers; } diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index 0f9ff3d7c2ef20..22c39212e36b0e 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -285,7 +285,7 @@ void TLVPacketBufferBackingStoreTest::TestWriterReserveUnreserveDoesNotOverflow( uint32_t lengthRemaining = writer.GetRemainingFreeLength(); NL_TEST_ASSERT(inSuite, lengthRemaining == 1); // Lets try to overflow by getting next buffer in the chain, - // unreserving then writting until the end of the current buffer. + // unreserving then writing until the end of the current buffer. error = writer.Put(TLV::AnonymousTag(), static_cast(7)); NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR); From 89c83ccc433cd740825d1858d58296be6cf54a54 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 1 Dec 2023 14:19:00 +0000 Subject: [PATCH 6/9] Small fix --- src/system/TLVPacketBufferBackingStore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system/TLVPacketBufferBackingStore.h b/src/system/TLVPacketBufferBackingStore.h index 6702cda259cfff..c39b871233ebd3 100644 --- a/src/system/TLVPacketBufferBackingStore.h +++ b/src/system/TLVPacketBufferBackingStore.h @@ -82,7 +82,7 @@ class TLVPacketBufferBackingStore : public chip::TLV::TLVBackingStore CHIP_ERROR FinalizeBuffer(chip::TLV::TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) override; virtual bool GetNewBufferWillAlwaysFail() override { - // For non-chained buffers, caller is given one chunk of contiguous memory all calls to + // For non-chained buffers, caller is given one chunk of contiguous memory. All calls to // GetNewBuffer will fail with CHIP_ERROR_NO_MEMORY. return !mUseChainedBuffers; } From 96760806e9bf8755a3f2e0b0b39fed453205d7f9 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 13 Dec 2023 16:03:44 +0000 Subject: [PATCH 7/9] Address PR Comment --- src/lib/core/TLVBackingStore.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lib/core/TLVBackingStore.h b/src/lib/core/TLVBackingStore.h index 1e232c24ed79f7..e7712a066b9010 100644 --- a/src/lib/core/TLVBackingStore.h +++ b/src/lib/core/TLVBackingStore.h @@ -156,6 +156,14 @@ class DLL_EXPORT TLVBackingStore */ virtual CHIP_ERROR FinalizeBuffer(TLVWriter & writer, uint8_t * bufStart, uint32_t bufLen) = 0; + /** + * Returns whether call to GetNewBuffer will always fail. + * + * There are some implementations of TLVBackingStore that provide some level of utility, such as access to pool + * of particular kind of buffer and/or reserving space for headers. Some implementation allow the caller to + * specify that they only intend to use a single buffer. It is useful for TLVWriter to know if this is the case. + * + */ virtual bool GetNewBufferWillAlwaysFail() { return false; } }; From c10695daba7d0cb18887a0d50490a6d50abc8c24 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 13 Dec 2023 16:13:58 +0000 Subject: [PATCH 8/9] Missing file save --- src/lib/core/TLVWriter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/core/TLVWriter.cpp b/src/lib/core/TLVWriter.cpp index 63e9eca47d607e..b20cc70adde8f0 100644 --- a/src/lib/core/TLVWriter.cpp +++ b/src/lib/core/TLVWriter.cpp @@ -130,6 +130,7 @@ CHIP_ERROR TLVWriter::Finalize() CHIP_ERROR TLVWriter::ReserveBuffer(uint32_t aBufferSize) { + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mRemainingLen >= aBufferSize, CHIP_ERROR_NO_MEMORY); if (mBackingStore) From 7fb1c88632a700516716182a453f1e40a0a60887 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 13 Dec 2023 16:36:13 -0500 Subject: [PATCH 9/9] Fix CI --- src/system/tests/TestTLVPacketBufferBackingStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/system/tests/TestTLVPacketBufferBackingStore.cpp b/src/system/tests/TestTLVPacketBufferBackingStore.cpp index 22c39212e36b0e..e46906d2edbbb3 100644 --- a/src/system/tests/TestTLVPacketBufferBackingStore.cpp +++ b/src/system/tests/TestTLVPacketBufferBackingStore.cpp @@ -257,7 +257,7 @@ void TLVPacketBufferBackingStoreTest::NonChainedBufferCanReserve(nlTestSuite * i uint32_t smallSize = 5; uint32_t smallerSizeToReserver = smallSize - 1; - auto buffer = PacketBufferHandle::New(smallSize, /* reservedSize = */ 0); + auto buffer = PacketBufferHandle::New(smallSize, /* aReservedSize = */ 0); PacketBufferTLVWriter writer; writer.Init(std::move(buffer), /* useChainedBuffers = */ false);