Skip to content

Commit

Permalink
Fix event buffer size boundary check (#25234)
Browse files Browse the repository at this point in the history
* improve event log dump

* Fix event buffer size boundary check
  • Loading branch information
yunhanw-google authored and pull[bot] committed Jul 3, 2023
1 parent 8504012 commit 2411970
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 9 deletions.
11 changes: 11 additions & 0 deletions src/app/EventManagement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/app/EventManagement.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<unsigned int>(elementCount));
chip::app::EventManagement::GetInstance().GetEventReader(reader, chip::app::PriorityLevel::Critical, &bufWrapper);
chip::TLV::Debug::Dump(reader, SimpleDumpWriter);
}

Expand Down
12 changes: 8 additions & 4 deletions src/lib/core/TLVCircularBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -213,8 +219,6 @@ CHIP_ERROR TLVCircularBuffer::GetNewBuffer(TLVWriter & ioWriter, uint8_t *& outB
{
outBufLen = static_cast<uint32_t>(mQueueHead - tail);
}

return CHIP_NO_ERROR;
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/lib/core/TLVCircularBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 2411970

Please sign in to comment.