Skip to content

Commit 094aadd

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. 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 #31779, #33307.
1 parent 1b455b5 commit 094aadd

12 files changed

+122
-64
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 <= System::PacketBuffer::kLargeBufMaxSize);
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/SystemConfig.h

+9
Original file line numberDiff line numberDiff line change
@@ -771,3 +771,12 @@ struct LwIPEvent;
771771
#define CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD 0
772772
#endif
773773
#endif // CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD
774+
775+
/**
776+
* @def CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
777+
*
778+
* @brief Maximum buffer allocation size of a 'Large' message
779+
*/
780+
#ifndef CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
781+
#define CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES (64000)
782+
#endif

src/system/SystemPacketBuffer.cpp

+14-12
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,11 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
514514
static_assert(PacketBuffer::kStructureSize == sizeof(PacketBuffer), "PacketBuffer size mismatch");
515515
static_assert(PacketBuffer::kStructureSize < UINT16_MAX, "Check for overflow more carefully");
516516
static_assert(SIZE_MAX >= INT_MAX, "Our additions might not fit in size_t");
517-
static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT32_MAX, "PacketBuffer may have size not fitting uint32_t");
517+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
518+
static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT32_MAX, "Max size for Large payload buffers");
519+
static_assert(PacketBuffer::kMaxSizeWithoutReserve < PacketBuffer::kLargeBufMaxSizeWithoutReserve,
520+
"Large buffer configuration should be greater than the conventional buffer limit");
521+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
518522
#if CHIP_SYSTEM_CONFIG_USE_LWIP
519523
// LwIP based APIs have a maximum buffer size of UINT16_MAX. Ensure that
520524
// limit is met during allocation.
@@ -530,8 +534,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
530534

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

533-
// TODO: Change the max to a lower value
534-
if (aAvailableSize > UINT32_MAX || lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT32_MAX)
537+
if (lAllocSize > PacketBuffer::kMaxAllocSize)
535538
{
536539
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
537540
return PacketBufferHandle();
@@ -593,18 +596,15 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
593596
PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aDataSize, size_t aAdditionalSize,
594597
uint16_t aReservedSize)
595598
{
596-
if (aDataSize > UINT16_MAX)
597-
{
598-
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
599-
return PacketBufferHandle();
600-
}
601599
// Since `aDataSize` fits in uint16_t, the sum `aDataSize + aAdditionalSize` will not overflow.
602600
// `New()` will only return a non-null buffer if the total allocation size does not overflow.
603601
PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize);
604602
if (buffer.mBuffer != nullptr)
605603
{
606604
memcpy(buffer.mBuffer->payload, aData, aDataSize);
607605
#if CHIP_SYSTEM_CONFIG_USE_LWIP
606+
// The VerifyOrDie() in the New() call catches buffer allocations greater
607+
// than UINT16_MAX for LwIP based platforms.
608608
buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
609609
#else
610610
buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
@@ -727,18 +727,20 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
727727
size_t originalDataSize = original->MaxDataLength();
728728
uint16_t originalReservedSize = original->ReservedSize();
729729

730-
if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
730+
uint32_t maxSize = PacketBuffer::kMaxAllocSize;
731+
732+
if (originalDataSize + originalReservedSize > maxSize)
731733
{
732734
// The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
733735
// and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
734736
// It is a genuine error if that extra space has been used.
735-
if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
737+
if (originalReservedSize + original->DataLength() > maxSize)
736738
{
737739
return PacketBufferHandle();
738740
}
739741
// 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;
742+
// guarantees originalReservedSize <= maxSize.
743+
originalDataSize = maxSize - originalReservedSize;
742744
}
743745

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

src/system/SystemPacketBuffer.h

+12
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,18 @@ class DLL_EXPORT PacketBuffer : private pbuf
138138
*/
139139
static constexpr size_t kMaxSize = kMaxSizeWithoutReserve - kDefaultHeaderReserve;
140140

141+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
142+
static constexpr size_t kLargeBufMaxSizeWithoutReserve = CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES;
143+
144+
static constexpr size_t kLargeBufMaxSize = kLargeBufMaxSizeWithoutReserve - kDefaultHeaderReserve;
145+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
146+
147+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
148+
static constexpr size_t kMaxAllocSize = kLargeBufMaxSizeWithoutReserve;
149+
#else
150+
static constexpr size_t kMaxAllocSize = kMaxSizeWithoutReserve;
151+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
152+
141153
/**
142154
* Return the size of the allocation including the reserved and payload data spaces but not including space
143155
* allocated for the PacketBuffer structure.

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_SYSTEM_CONFIG_MAX_LARGE_BUFFER_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_SYSTEM_CONFIG_MAX_LARGE_BUFFER_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_SYSTEM_CONFIG_MAX_LARGE_BUFFER_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_SYSTEM_CONFIG_MAX_LARGE_BUFFER_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/SecureMessageCodec.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView
4141
{
4242
VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
4343
VerifyOrReturnError(!msgBuf->HasChainedBuffer(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
44-
VerifyOrReturnError(msgBuf->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
4544

4645
ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(msgBuf));
4746

src/transport/SessionManager.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,17 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
201201
packetHeader.SetSecureSessionControlMsg(true);
202202
}
203203

204+
if (sessionHandle->AllowsMRP())
205+
{
206+
VerifyOrReturnError(message->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
207+
}
208+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
209+
else if (sessionHandle->AllowsLargePayload())
210+
{
211+
VerifyOrReturnError(message->TotalLength() <= kMaxLargeAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
212+
}
213+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
214+
204215
#if CHIP_PROGRESS_LOGGING
205216
NodeId destination;
206217
FabricIndex fabricIndex;

src/transport/raw/MessageHeader.h

+11-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ static constexpr size_t kMaxPacketBufferApplicationPayloadAndMICSizeBytes = Syst
6060

6161
static constexpr size_t kMaxApplicationPayloadAndMICSizeBytes =
6262
min(kMaxPerSpecApplicationPayloadAndMICSizeBytes, kMaxPacketBufferApplicationPayloadAndMICSizeBytes);
63-
6463
} // namespace detail
6564

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

7574
static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;
7675

76+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
77+
// Minimum header size of TCP + IPv6 without options.
78+
static constexpr size_t kMaxTCPAndIPHeaderSizeBytes = 60;
79+
80+
// Max space for the Application Payload and MIC for large packet buffers
81+
// This is the size _excluding_ the header reserve.
82+
static constexpr size_t kMaxLargeApplicationPayloadAndMICSizeBytes = System::PacketBuffer::kLargeBufMaxSize - kMaxTCPAndIPHeaderSizeBytes;
83+
84+
static constexpr size_t kMaxLargeAppMessageLen = kMaxLargeApplicationPayloadAndMICSizeBytes - kMaxTagLen;
85+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
86+
7787
typedef int PacketHeaderFlags;
7888

7989
namespace Header {

src/transport/raw/TCP.cpp

+9-10
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ 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

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

4847
constexpr int kListenBacklogSize = 2;
4948

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

203202
VerifyOrReturnError(address.GetTransportType() == Type::kTcp, CHIP_ERROR_INVALID_ARGUMENT);
204203
VerifyOrReturnError(mState == TCPState::kInitialized, CHIP_ERROR_INCORRECT_STATE);
205-
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint16_t>::max(),
204+
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint32_t>::max(),
206205
CHIP_ERROR_INVALID_ARGUMENT);
207206

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

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

216215
// Reuse existing connection if one exists, otherwise a new one
217216
// will be established
@@ -324,11 +323,11 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
324323
{
325324
return err;
326325
}
327-
uint16_t messageSize = LittleEndian::Get16(messageSizeBuf);
328-
if (messageSize >= kMaxMessageSize)
326+
uint32_t messageSize = LittleEndian::Get32(messageSizeBuf);
327+
if (messageSize >= kMaxTCPMessageSize)
329328
{
330-
331-
// This message is too long for upper layers.
329+
// Message is too big for node to process. Disconnect with peer.
330+
CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No);
332331
return CHIP_ERROR_MESSAGE_TOO_LONG;
333332
}
334333
// The subtraction will not underflow because we successfully read kPacketSizeBytes.
@@ -344,7 +343,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
344343
return CHIP_NO_ERROR;
345344
}
346345

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

src/transport/raw/TCP.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ class DLL_EXPORT TCPBase : public Base
258258
* is no other data).
259259
* @param[in] messageSize Size of the single message.
260260
*/
261-
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize);
261+
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize);
262262

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

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

313313
// Number of active and 'pending connection' endpoints
314314
size_t mUsedEndPointCount = 0;

src/transport/raw/TCPConfig.h

-9
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,6 @@ namespace chip {
6363
#define CHIP_CONFIG_MAX_TCP_PENDING_PACKETS 4
6464
#endif
6565

66-
/**
67-
* @def CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
68-
*
69-
* @brief Maximum payload size of a message over a TCP connection
70-
*/
71-
#ifndef CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
72-
#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES 1000000
73-
#endif
74-
7566
/**
7667
* @def CHIP_CONFIG_TCP_CONNECT_TIMEOUT_MSECS
7768
*

0 commit comments

Comments
 (0)