Skip to content

Commit

Permalink
Ensure that stack layers above transport manager never get chained pa…
Browse files Browse the repository at this point in the history
…cket buffers.

None of our upper-layer core really deals with chained packet buffers,
starting with the way we do in-place decryption that assumes that we
have a contiguous buffer.

The changes here:

1) Flatten out possibly-chained buffers in the LwIP case in the UDP
   endpoint.
2) Ensure that we don't have a chained buffer in BTP.  There was
   already a check for this in the "in progress" case; this change just
   adds the same check in the "idle" case.
  • Loading branch information
bzbarsky-apple committed Sep 10, 2021
1 parent 2addf00 commit b8423e2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/ble/BtpEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&

mRxBuf->AddToEnd(std::move(data));
mRxBuf->CompactHead(); // will free 'data' and adjust rx buf's end/length

// For now, limit BtpEngine message size to max length of 1 pbuf, as we do for chip messages sent via IP.
// TODO add support for BtpEngine messages longer than 1 pbuf
VerifyOrExit(!mRxBuf->HasChainedBuffer(), err = CHIP_ERROR_INBOUND_MESSAGE_TOO_BIG);
}
else if (mRxState == kState_InProgress)
{
Expand Down
18 changes: 18 additions & 0 deletions src/inet/UDPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,24 @@ void UDPEndPoint::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb, struct
System::LayerLwIP * lSystemLayer = static_cast<System::LayerLwIP *>(ep->Layer().SystemLayer());
IPPacketInfo * pktInfo = NULL;
System::PacketBufferHandle buf = System::PacketBufferHandle::Adopt(p);
if (buf->HasChainedBuffer())
{
// Try the simple expedient of flattening in-place.
buf->CompactHead();
}

if (buf->HasChainedBuffer())
{
// Have to allocate a new big-enough buffer and copy.
uint16_t messageSize = buf->TotalLength();
System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
{
ChipLogError(Inet, "No memory to flatten incoming packet buffer chain of size %" PRIu16, buf->TotalLength());
return;
}
buf = std::move(copy);
}

pktInfo = GetPacketInfo(buf);
if (pktInfo != NULL)
Expand Down
9 changes: 9 additions & 0 deletions src/transport/TransportMgrBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ void TransportMgrBase::Close()

void TransportMgrBase::HandleMessageReceived(const Transport::PeerAddress & peerAddress, System::PacketBufferHandle && msg)
{
if (msg->HasChainedBuffer())
{
// Something in the lower levels messed up.
char addrBuffer[Transport::PeerAddress::kMaxToStringSize];
peerAddress.ToString(addrBuffer);
ChipLogError(Inet, "message from %s dropped due to lower layers not ensuring a single packet buffer.", addrBuffer);
return;
}

if (mSecureSessionMgr != nullptr)
{
mSecureSessionMgr->OnMessageReceived(peerAddress, std::move(msg));
Expand Down

0 comments on commit b8423e2

Please sign in to comment.