Skip to content

Commit

Permalink
Changes for large Packetbuffer allocation to support TCP payloads
Browse files Browse the repository at this point in the history
Changes to internal checks in SystemPacketBuffer.

Update the length encoding for TCP payloads during send and receive.

Max size config for large packetbuffer size limit in SystemPacketBuffer.h.

Define Max application payload size for large buffers

Test modifications for chainedbuffer receives for TCP.
- Add test for Buffer length greater than MRP max size.

Fixes Issues project-chip#31779, project-chip#33307.
  • Loading branch information
pidarped committed May 26, 2024
1 parent 3718e99 commit fa93f3d
Show file tree
Hide file tree
Showing 13 changed files with 122 additions and 68 deletions.
2 changes: 1 addition & 1 deletion src/inet/TCPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
/**
* Size of the largest TCP packet that can be received.
*/
constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxSizeWithoutReserve;
constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kLargeBufMaxSizeWithoutReserve;

protected:
friend class TCPTest;
Expand Down
5 changes: 2 additions & 3 deletions src/inet/TCPEndPointImplSockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -947,10 +947,9 @@ void TCPEndPointImplSockets::ReceiveData()
{
VerifyOrDie(rcvLen > 0);
size_t newDataLength = rcvBuf->DataLength() + static_cast<size_t>(rcvLen);
VerifyOrDie(CanCastTo<uint16_t>(newDataLength));
if (isNewBuf)
{
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength));
rcvBuf->SetDataLength(newDataLength);
rcvBuf.RightSize();
if (mRcvQueue.IsNull())
{
Expand All @@ -963,7 +962,7 @@ void TCPEndPointImplSockets::ReceiveData()
}
else
{
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength), mRcvQueue);
rcvBuf->SetDataLength(newDataLength, mRcvQueue);
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/system/SystemConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -788,3 +788,12 @@ struct LwIPEvent;
#define CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD 0
#endif
#endif // CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD

/**
* @def CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
*
* @brief Maximum buffer allocation size of a 'Large' message
*/
#ifndef CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
#define CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES (64000)
#endif
34 changes: 24 additions & 10 deletions src/system/SystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,21 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
// Setting a static upper bound on the maximum buffer size allocation for regular sized messages (not large).
static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT16_MAX, "kMaxSizeWithoutReserve should not exceed UINT16_MAX.");

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
// Setting a static upper bound on the maximum buffer size allocation for
// large messages.
#if CHIP_SYSTEM_CONFIG_USE_LWIP
static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT16_MAX, "In LwIP, max size for Large payload buffers cannot exceed UINT16_MAX!");
#else
static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT32_MAX, "Max size for Large payload buffers cannot exceed UINT32_MAX");
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

// Ensure that the definition of the max buffer allocation for regular
// messages is less than that for large ones.
static_assert(PacketBuffer::kMaxSizeWithoutReserve < PacketBuffer::kLargeBufMaxSizeWithoutReserve,
"Large buffer configuration should be greater than the conventional buffer limit");
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

// Ensure that aAvailableSize is bound within a max and is not big enough to cause overflow during
// subsequent addition of all the sizes.
if (aAvailableSize > UINT32_MAX)
Expand Down Expand Up @@ -557,7 +572,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese

CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_PacketBufferNew, return PacketBufferHandle());

if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve)
if (lAllocSize > PacketBuffer::kMaxAllocSize)
{
ChipLogError(chipSystemLayer, "PacketBuffer: allocation exceeding buffer capacity limits.");
return PacketBufferHandle();
Expand Down Expand Up @@ -621,18 +636,15 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aDataSize, size_t aAdditionalSize,
uint16_t aReservedSize)
{
if (aDataSize > UINT16_MAX)
{
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
return PacketBufferHandle();
}
// Since `aDataSize` fits in uint16_t, the sum `aDataSize + aAdditionalSize` will not overflow.
// `New()` will only return a non-null buffer if the total allocation size does not overflow.
PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize);
if (buffer.mBuffer != nullptr)
{
memcpy(buffer.mBuffer->payload, aData, aDataSize);
#if CHIP_SYSTEM_CONFIG_USE_LWIP
// The VerifyOrDie() in the New() call catches buffer allocations greater
// than UINT16_MAX for LwIP based platforms.
buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
#else
buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
Expand Down Expand Up @@ -755,18 +767,20 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
size_t originalDataSize = original->MaxDataLength();
uint16_t originalReservedSize = original->ReservedSize();

if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
uint32_t maxSize = PacketBuffer::kMaxAllocSize;

if (originalDataSize + originalReservedSize > maxSize)
{
// The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
// and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
// It is a genuine error if that extra space has been used.
if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
if (originalReservedSize + original->DataLength() > maxSize)
{
return PacketBufferHandle();
}
// Otherwise, reduce the requested data size. This subtraction can not underflow because the above test
// guarantees originalReservedSize <= PacketBuffer::kMaxSizeWithoutReserve.
originalDataSize = PacketBuffer::kMaxSizeWithoutReserve - originalReservedSize;
// guarantees originalReservedSize <= maxSize.
originalDataSize = maxSize - originalReservedSize;
}

PacketBufferHandle clone = PacketBufferHandle::New(originalDataSize, originalReservedSize);
Expand Down
12 changes: 12 additions & 0 deletions src/system/SystemPacketBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,18 @@ class DLL_EXPORT PacketBuffer : private pbuf
*/
static constexpr size_t kMaxSize = kMaxSizeWithoutReserve - kDefaultHeaderReserve;

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
static constexpr size_t kLargeBufMaxSizeWithoutReserve = CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES;

static constexpr size_t kLargeBufMaxSize = kLargeBufMaxSizeWithoutReserve - kDefaultHeaderReserve;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
static constexpr size_t kMaxAllocSize = kLargeBufMaxSizeWithoutReserve;
#else
static constexpr size_t kMaxAllocSize = kMaxSizeWithoutReserve;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

/**
* Return the size of the allocation including the reserved and payload data spaces but not including space
* allocated for the PacketBuffer structure.
Expand Down
18 changes: 10 additions & 8 deletions src/system/tests/TestSystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckNew)
{
const PacketBufferHandle buffer = PacketBufferHandle::New(0, config.reserved_size);

if (config.reserved_size > PacketBuffer::kMaxSizeWithoutReserve)
if (config.reserved_size > PacketBuffer::kMaxAllocSize)
{
EXPECT_TRUE(buffer.IsNull());
continue;
Expand Down Expand Up @@ -1605,7 +1605,8 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleRightSize)

TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
{
uint8_t lPayload[2 * PacketBuffer::kMaxSizeWithoutReserve];
uint8_t lPayload[2 * PacketBuffer::kMaxAllocSize];

for (uint8_t & payload : lPayload)
{
payload = static_cast<uint8_t>(random());
Expand Down Expand Up @@ -1684,7 +1685,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
// This is only testable on heap allocation configurations, where pbuf records the allocation size and we can manually
// construct an oversize buffer.

constexpr uint16_t kOversizeDataSize = PacketBuffer::kMaxSizeWithoutReserve + 99;
constexpr size_t kOversizeDataSize = PacketBuffer::kMaxAllocSize + 99;
PacketBuffer * p = reinterpret_cast<PacketBuffer *>(chip::Platform::MemoryAlloc(kStructureSize + kOversizeDataSize));
ASSERT_NE(p, nullptr);

Expand All @@ -1698,15 +1699,16 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
PacketBufferHandle handle = PacketBufferHandle::Adopt(p);

// Fill the buffer to maximum and verify that it can be cloned.
size_t maxSize = PacketBuffer::kMaxAllocSize;

memset(handle->Start(), 1, PacketBuffer::kMaxSizeWithoutReserve);
handle->SetDataLength(PacketBuffer::kMaxSizeWithoutReserve);
EXPECT_EQ(handle->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
memset(handle->Start(), 1, maxSize);
handle->SetDataLength(maxSize);
EXPECT_EQ(handle->DataLength(), maxSize);

PacketBufferHandle clone = handle.CloneData();
ASSERT_FALSE(clone.IsNull());
EXPECT_EQ(clone->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), PacketBuffer::kMaxSizeWithoutReserve), 0);
EXPECT_EQ(clone->DataLength(), maxSize);
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), maxSize), 0);

// Overfill the buffer and verify that it can not be cloned.
memset(handle->Start(), 2, kOversizeDataSize);
Expand Down
1 change: 0 additions & 1 deletion src/transport/SecureMessageCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView
{
VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(!msgBuf->HasChainedBuffer(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
VerifyOrReturnError(msgBuf->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);

ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(msgBuf));

Expand Down
11 changes: 11 additions & 0 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,17 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
packetHeader.SetSecureSessionControlMsg(true);
}

if (sessionHandle->AllowsMRP())
{
VerifyOrReturnError(message->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
}
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
else if (sessionHandle->AllowsLargePayload())
{
VerifyOrReturnError(message->TotalLength() <= kMaxLargeAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
}
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

#if CHIP_PROGRESS_LOGGING
NodeId destination;
FabricIndex fabricIndex;
Expand Down
12 changes: 11 additions & 1 deletion src/transport/raw/MessageHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ static constexpr size_t kMaxPacketBufferApplicationPayloadAndMICSizeBytes = Syst

static constexpr size_t kMaxApplicationPayloadAndMICSizeBytes =
min(kMaxPerSpecApplicationPayloadAndMICSizeBytes, kMaxPacketBufferApplicationPayloadAndMICSizeBytes);

} // namespace detail

static constexpr size_t kMaxTagLen = 16;
Expand All @@ -74,6 +73,17 @@ static constexpr size_t kMaxAppMessageLen = detail::kMaxApplicationPayloadAndMIC

static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;

#if INET_CONFIG_ENABLE_TCP_ENDPOINT
// Minimum header size of TCP + IPv6 without options.
static constexpr size_t kMaxTCPAndIPHeaderSizeBytes = 60;

// Max space for the Application Payload and MIC for large packet buffers
// This is the size _excluding_ the header reserve.
static constexpr size_t kMaxLargeApplicationPayloadAndMICSizeBytes = System::PacketBuffer::kLargeBufMaxSize - kMaxTCPAndIPHeaderSizeBytes;

static constexpr size_t kMaxLargeAppMessageLen = kMaxLargeApplicationPayloadAndMICSizeBytes - kMaxTagLen;
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT

typedef int PacketHeaderFlags;

namespace Header {
Expand Down
23 changes: 11 additions & 12 deletions src/transport/raw/TCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ namespace {

using namespace chip::Encoding;

// Packets start with a 16-bit size
constexpr size_t kPacketSizeBytes = 2;
// Packets start with a 32-bit size field.
constexpr size_t kPacketSizeBytes = sizeof(uint32_t);

// TODO: Actual limit may be lower (spec issue #2119)
constexpr uint16_t kMaxMessageSize = static_cast<uint16_t>(System::PacketBuffer::kMaxSizeWithoutReserve - kPacketSizeBytes);
constexpr uint32_t kMaxTCPMessageSize = static_cast<uint32_t>(System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - kPacketSizeBytes);

constexpr int kListenBacklogSize = 2;

Expand Down Expand Up @@ -197,12 +196,12 @@ ActiveTCPConnectionState * TCPBase::FindInUseConnection(const Inet::TCPEndPoint
CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::PacketBufferHandle && msgBuf)
{
// Sent buffer data format is:
// - packet size as a uint16_t
// - packet size as a uint32_t
// - actual data

VerifyOrReturnError(address.GetTransportType() == Type::kTcp, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(mState == TCPState::kInitialized, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint16_t>::max(),
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= System::PacketBuffer::kLargeBufMaxSizeWithoutReserve,
CHIP_ERROR_INVALID_ARGUMENT);

// The check above about kPacketSizeBytes + msgBuf->DataLength() means it definitely fits in uint16_t.
Expand All @@ -211,7 +210,7 @@ CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::
msgBuf->SetStart(msgBuf->Start() - kPacketSizeBytes);

uint8_t * output = msgBuf->Start();
LittleEndian::Write16(output, static_cast<uint16_t>(msgBuf->DataLength() - kPacketSizeBytes));
LittleEndian::Write32(output, static_cast<uint32_t>(msgBuf->DataLength() - kPacketSizeBytes));

// Reuse existing connection if one exists, otherwise a new one
// will be established
Expand Down Expand Up @@ -324,11 +323,11 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
{
return err;
}
uint16_t messageSize = LittleEndian::Get16(messageSizeBuf);
if (messageSize >= kMaxMessageSize)
uint32_t messageSize = LittleEndian::Get32(messageSizeBuf);
if (messageSize >= kMaxTCPMessageSize)
{

// This message is too long for upper layers.
// Message is too big for node to process. Disconnect with peer.
CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No);
return CHIP_ERROR_MESSAGE_TOO_LONG;
}
// The subtraction will not underflow because we successfully read kPacketSizeBytes.
Expand All @@ -344,7 +343,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
return CHIP_NO_ERROR;
}

CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize)
CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize)
{
// We enter with `state->mReceived` containing at least one full message, perhaps in a chain.
// `state->mReceived->Start()` currently points to the message data.
Expand Down
4 changes: 2 additions & 2 deletions src/transport/raw/TCP.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class DLL_EXPORT TCPBase : public Base
* is no other data).
* @param[in] messageSize Size of the single message.
*/
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize);
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize);

/**
* Initiate a connection to the given peer. On connection completion,
Expand Down Expand Up @@ -308,7 +308,7 @@ class DLL_EXPORT TCPBase : public Base

// The max payload size of data over a TCP connection that is transmissible
// at a time.
uint32_t mMaxTCPPayloadSize = CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES;
uint32_t mMaxTCPSendAllocSize = System::PacketBuffer::kMaxAllocSize;

// Number of active and 'pending connection' endpoints
size_t mUsedEndPointCount = 0;
Expand Down
9 changes: 0 additions & 9 deletions src/transport/raw/TCPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ namespace chip {
#define CHIP_CONFIG_MAX_TCP_PENDING_PACKETS 4
#endif

/**
* @def CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
*
* @brief Maximum payload size of a message over a TCP connection
*/
#ifndef CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES 1000000
#endif

/**
* @def CHIP_CONFIG_TCP_CONNECT_TIMEOUT_MSECS
*
Expand Down
Loading

0 comments on commit fa93f3d

Please sign in to comment.