Skip to content

Commit 6dd1c8e

Browse files
committed
Changes for large Packetbuffer allocation to support TCP payloads
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. Test modifications for chainedbuffer receives for TCP. - Add test for Buffer length greater than MRP max size. Fixes Issues #31779, #33307.
1 parent a8cfbac commit 6dd1c8e

8 files changed

+105
-48
lines changed

src/inet/TCPEndPointImplSockets.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -947,10 +947,10 @@ void TCPEndPointImplSockets::ReceiveData()
947947
{
948948
VerifyOrDie(rcvLen > 0);
949949
size_t newDataLength = rcvBuf->DataLength() + static_cast<size_t>(rcvLen);
950-
VerifyOrDie(CanCastTo<uint16_t>(newDataLength));
950+
VerifyOrDie(newDataLength <= UINT32_MAX);
951951
if (isNewBuf)
952952
{
953-
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength));
953+
rcvBuf->SetDataLength(newDataLength);
954954
rcvBuf.RightSize();
955955
if (mRcvQueue.IsNull())
956956
{
@@ -963,7 +963,7 @@ void TCPEndPointImplSockets::ReceiveData()
963963
}
964964
else
965965
{
966-
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength), mRcvQueue);
966+
rcvBuf->SetDataLength(newDataLength, mRcvQueue);
967967
}
968968
}
969969
}

src/system/SystemPacketBuffer.cpp

+25-6
Original file line numberDiff line numberDiff line change
@@ -530,12 +530,19 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
530530

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

533-
// TODO: Change the max to a lower value
534-
if (aAvailableSize > UINT32_MAX || lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT32_MAX)
533+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
534+
if (lAllocSize > CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES || lBlockSize > UINT32_MAX)
535+
{
536+
ChipLogError(chipSystemLayer, "PacketBuffer: allocation exceeds limit for large payload size.");
537+
return PacketBufferHandle();
538+
}
539+
#else
540+
if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT16_MAX)
535541
{
536542
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
537543
return PacketBufferHandle();
538544
}
545+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
539546

540547
#if CHIP_SYSTEM_CONFIG_USE_LWIP
541548

@@ -593,7 +600,11 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
593600
PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aDataSize, size_t aAdditionalSize,
594601
uint16_t aReservedSize)
595602
{
603+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
604+
if (aDataSize > UINT32_MAX)
605+
#else
596606
if (aDataSize > UINT16_MAX)
607+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
597608
{
598609
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
599610
return PacketBufferHandle();
@@ -605,6 +616,8 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD
605616
{
606617
memcpy(buffer.mBuffer->payload, aData, aDataSize);
607618
#if CHIP_SYSTEM_CONFIG_USE_LWIP
619+
// The VerifyOrDie() in the New() call catches buffer allocations greater
620+
// than UINT16_MAX for LwIP based platforms.
608621
buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
609622
#else
610623
buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
@@ -727,18 +740,24 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
727740
size_t originalDataSize = original->MaxDataLength();
728741
uint16_t originalReservedSize = original->ReservedSize();
729742

730-
if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
743+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
744+
uint32_t maxSize = CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES;
745+
#else
746+
uint32_t maxSize = PacketBuffer::kMaxSizeWithoutReserve;
747+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
748+
749+
if (originalDataSize + originalReservedSize > maxSize)
731750
{
732751
// The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
733752
// and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
734753
// It is a genuine error if that extra space has been used.
735-
if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
754+
if (originalReservedSize + original->DataLength() > maxSize)
736755
{
737756
return PacketBufferHandle();
738757
}
739758
// Otherwise, reduce the requested data size. This subtraction can not underflow because the above test
740-
// guarantees originalReservedSize <= PacketBuffer::kMaxSizeWithoutReserve.
741-
originalDataSize = PacketBuffer::kMaxSizeWithoutReserve - originalReservedSize;
759+
// guarantees originalReservedSize <= maxSize.
760+
originalDataSize = maxSize - originalReservedSize;
742761
}
743762

744763
PacketBufferHandle clone = PacketBufferHandle::New(originalDataSize, originalReservedSize);

src/system/SystemPacketBuffer.h

+13
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,19 @@
4444
#include <lwip/pbuf.h>
4545
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
4646

47+
/**
48+
* @def CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES
49+
*
50+
* @brief Maximum payload size of a 'Large' message
51+
*/
52+
#ifndef CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES
53+
#define CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES (64000)
54+
#endif
55+
56+
#if CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES > UINT32_MAX
57+
#error "The maximum Large payload size cannot exceed UINT32_MAX"
58+
#endif
59+
4760
namespace chip {
4861
namespace System {
4962

src/system/tests/TestSystemPacketBuffer.cpp

+22-5
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,11 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckNew)
309309
{
310310
const PacketBufferHandle buffer = PacketBufferHandle::New(0, config.reserved_size);
311311

312+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
313+
if (config.reserved_size > CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES)
314+
#else
312315
if (config.reserved_size > PacketBuffer::kMaxSizeWithoutReserve)
316+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
313317
{
314318
EXPECT_TRUE(buffer.IsNull());
315319
continue;
@@ -1605,7 +1609,11 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleRightSize)
16051609

16061610
TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16071611
{
1612+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
1613+
uint8_t lPayload[2 * CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES];
1614+
#else
16081615
uint8_t lPayload[2 * PacketBuffer::kMaxSizeWithoutReserve];
1616+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
16091617
for (uint8_t & payload : lPayload)
16101618
{
16111619
payload = static_cast<uint8_t>(random());
@@ -1684,7 +1692,11 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16841692
// This is only testable on heap allocation configurations, where pbuf records the allocation size and we can manually
16851693
// construct an oversize buffer.
16861694

1695+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
1696+
constexpr uint32_t kOversizeDataSize = CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES + 99;
1697+
#else
16871698
constexpr uint16_t kOversizeDataSize = PacketBuffer::kMaxSizeWithoutReserve + 99;
1699+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
16881700
PacketBuffer * p = reinterpret_cast<PacketBuffer *>(chip::Platform::MemoryAlloc(kStructureSize + kOversizeDataSize));
16891701
ASSERT_NE(p, nullptr);
16901702

@@ -1698,15 +1710,20 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16981710
PacketBufferHandle handle = PacketBufferHandle::Adopt(p);
16991711

17001712
// Fill the buffer to maximum and verify that it can be cloned.
1713+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
1714+
uint32_t maxSize = CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES;
1715+
#else
1716+
uint32_t maxSize = PacketBuffer::kMaxSizeWithoutReserve;
1717+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
17011718

1702-
memset(handle->Start(), 1, PacketBuffer::kMaxSizeWithoutReserve);
1703-
handle->SetDataLength(PacketBuffer::kMaxSizeWithoutReserve);
1704-
EXPECT_EQ(handle->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
1719+
memset(handle->Start(), 1, maxSize);
1720+
handle->SetDataLength(maxSize);
1721+
EXPECT_EQ(handle->DataLength(), maxSize);
17051722

17061723
PacketBufferHandle clone = handle.CloneData();
17071724
ASSERT_FALSE(clone.IsNull());
1708-
EXPECT_EQ(clone->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
1709-
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), PacketBuffer::kMaxSizeWithoutReserve), 0);
1725+
EXPECT_EQ(clone->DataLength(), maxSize);
1726+
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), maxSize), 0);
17101727

17111728
// Overfill the buffer and verify that it can not be cloned.
17121729
memset(handle->Start(), 2, kOversizeDataSize);

src/transport/raw/TCP.cpp

+9-9
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ namespace {
4040
using namespace chip::Encoding;
4141

4242
// Packets start with a 16-bit size
43-
constexpr size_t kPacketSizeBytes = 2;
43+
constexpr size_t kPacketSizeBytes = sizeof(uint32_t);
4444

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

4848
constexpr int kListenBacklogSize = 2;
4949

@@ -202,7 +202,7 @@ CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::
202202

203203
VerifyOrReturnError(address.GetTransportType() == Type::kTcp, CHIP_ERROR_INVALID_ARGUMENT);
204204
VerifyOrReturnError(mState == TCPState::kInitialized, CHIP_ERROR_INCORRECT_STATE);
205-
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint16_t>::max(),
205+
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint32_t>::max(),
206206
CHIP_ERROR_INVALID_ARGUMENT);
207207

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

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

216216
// Reuse existing connection if one exists, otherwise a new one
217217
// will be established
@@ -324,11 +324,11 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
324324
{
325325
return err;
326326
}
327-
uint16_t messageSize = LittleEndian::Get16(messageSizeBuf);
328-
if (messageSize >= kMaxMessageSize)
327+
uint32_t messageSize = LittleEndian::Get32(messageSizeBuf);
328+
if (messageSize >= kMaxTCPMessageSize)
329329
{
330-
331-
// This message is too long for upper layers.
330+
// Message is too big for node to process. Disconnect with peer.
331+
CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No);
332332
return CHIP_ERROR_MESSAGE_TOO_LONG;
333333
}
334334
// The subtraction will not underflow because we successfully read kPacketSizeBytes.
@@ -344,7 +344,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
344344
return CHIP_NO_ERROR;
345345
}
346346

347-
CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize)
347+
CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize)
348348
{
349349
// We enter with `state->mReceived` containing at least one full message, perhaps in a chain.
350350
// `state->mReceived->Start()` currently points to the message data.

src/transport/raw/TCP.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ class DLL_EXPORT TCPBase : public Base
252252
* is no other data).
253253
* @param[in] messageSize Size of the single message.
254254
*/
255-
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize);
255+
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize);
256256

257257
/**
258258
* Initiate a connection to the given peer. On connection completion,

src/transport/raw/TCPConfig.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ namespace chip {
6969
* @brief Maximum payload size of a message over a TCP connection
7070
*/
7171
#ifndef CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
72-
#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES 1000000
72+
#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES (CHIP_CONFIG_MAX_LARGE_PAYLOAD_SIZE_BYTES)
7373
#endif
7474

7575
/**

0 commit comments

Comments
 (0)