From c8870d5f0425c43949d296376dd8e8657dadb212 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Wed, 22 Feb 2023 15:28:07 -0800 Subject: [PATCH] Fix event buffer size boundary check (#25234) * improve event log dump * Fix event buffer size boundary check --- src/app/EventManagement.cpp | 11 +++++++++++ src/app/EventManagement.h | 2 ++ src/app/tests/TestEventLogging.cpp | 6 +----- src/lib/core/TLVCircularBuffer.cpp | 12 ++++++++---- src/lib/core/TLVCircularBuffer.h | 11 +++++++++++ 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index a4777e0b1b471b..9fb66767ba7848 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -858,6 +858,17 @@ bool CircularEventBuffer::IsFinalDestinationForPriority(PriorityLevel aPriority) return !((mpNext != nullptr) && (mpNext->mPriority <= aPriority)); } +/** + * @brief + * TLVCircularBuffer::OnInit can modify the state of the buffer, but we don't want that behavior here. + * We want to make sure we don't change our state, and just report the currently-available space. + */ +CHIP_ERROR CircularEventBuffer::OnInit(TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) +{ + GetCurrentWritableBuffer(bufStart, bufLen); + return CHIP_NO_ERROR; +} + void CircularEventReader::Init(CircularEventBufferWrapper * apBufWrapper) { CircularEventBuffer * prev; diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 4a07ae341b1fe6..f4d575464df611 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -109,6 +109,8 @@ class CircularEventBuffer : public TLV::TLVCircularBuffer ///< lesser priority are dropped when they get bumped out of this buffer size_t mRequiredSpaceForEvicted = 0; ///< Required space for previous buffer to evict event to new buffer + + CHIP_ERROR OnInit(TLV::TLVWriter & writer, uint8_t *& bufStart, uint32_t & bufLen) override; }; class CircularEventReader; diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp index a3e1abe03b8b95..3653e9751ec11c 100644 --- a/src/app/tests/TestEventLogging.cpp +++ b/src/app/tests/TestEventLogging.cpp @@ -115,12 +115,8 @@ void ENFORCE_FORMAT(1, 2) SimpleDumpWriter(const char * aFormat, ...) void PrintEventLog() { chip::TLV::TLVReader reader; - size_t elementCount; chip::app::CircularEventBufferWrapper bufWrapper; - chip::app::EventManagement::GetInstance().GetEventReader(reader, chip::app::PriorityLevel::Debug, &bufWrapper); - - chip::TLV::Utilities::Count(reader, elementCount, false); - printf("Found %u elements\n", static_cast(elementCount)); + chip::app::EventManagement::GetInstance().GetEventReader(reader, chip::app::PriorityLevel::Critical, &bufWrapper); chip::TLV::Debug::Dump(reader, SimpleDumpWriter); } diff --git a/src/lib/core/TLVCircularBuffer.cpp b/src/lib/core/TLVCircularBuffer.cpp index 3f40d7d8bcdcc1..c88892fe22bfd5 100644 --- a/src/lib/core/TLVCircularBuffer.cpp +++ b/src/lib/core/TLVCircularBuffer.cpp @@ -194,14 +194,20 @@ CHIP_ERROR TLVCircularBuffer::OnInit(TLVWriter & writer, uint8_t *& bufStart, ui CHIP_ERROR TLVCircularBuffer::GetNewBuffer(TLVWriter & ioWriter, uint8_t *& outBufStart, uint32_t & outBufLen) { - uint8_t * tail = QueueTail(); - if (mQueueLength >= mQueueSize) { // Queue is out of space, need to evict an element ReturnErrorOnFailure(EvictHead()); } + GetCurrentWritableBuffer(outBufStart, outBufLen); + return CHIP_NO_ERROR; +} + +void TLVCircularBuffer::GetCurrentWritableBuffer(uint8_t *& outBufStart, uint32_t & outBufLen) const +{ + uint8_t * tail = QueueTail(); + // set the output values, returned buffer must be contiguous outBufStart = tail; @@ -213,8 +219,6 @@ CHIP_ERROR TLVCircularBuffer::GetNewBuffer(TLVWriter & ioWriter, uint8_t *& outB { outBufLen = static_cast(mQueueHead - tail); } - - return CHIP_NO_ERROR; } /** diff --git a/src/lib/core/TLVCircularBuffer.h b/src/lib/core/TLVCircularBuffer.h index a4eff110058746..80897b3267913e 100644 --- a/src/lib/core/TLVCircularBuffer.h +++ b/src/lib/core/TLVCircularBuffer.h @@ -128,6 +128,17 @@ class DLL_EXPORT TLVCircularBuffer : public chip::TLV::TLVBackingStore circular buffer. See the ProcessEvictedElementFunct type definition on additional information on implementing the mProcessEvictedElement function. */ +protected: + /** + * @brief + * returns the actual state of what our current available buffer space is + * + * @param[out] outBufStart The pointer to the current buffer + * + * @param[out] outBufLen The available length for writing + */ + void GetCurrentWritableBuffer(uint8_t *& outBufStart, uint32_t & outBufLen) const; + private: uint8_t * mQueue; uint32_t mQueueSize;