Re-alignment logic in LwIPReceiveUDPMessage() makes incorrect assumptions

[tcarmelveilleux]

Problem

In UDPEndPointImplLwIP::GetPacketInfo, there is a check of if (!aBuffer->EnsureReservedSize(sizeof(IPPacketInfo) + 3)) which implies desire to re-align. However, the code makes assumptions about the remaining use of the information consumed until now.

GetPacketInfo() returns a pointer to "loose data" before the actual payload, which is first gotten to set the info.

It is first used in the chain in UDPEndPointImplLwIP::LwIPReceiveUDPMessage():

   pktInfo = GetPacketInfo(buf);   // <------ Get pointer to front of data no longer used
    if (pktInfo != nullptr)   // <---- Reuse the space to shoe-horn ipInfo
    {
        pktInfo->SrcAddress  = IPAddress(*addr);
        pktInfo->DestAddress = IPAddress(*ip_current_dest_addr());
        pktInfo->Interface   = InterfaceId(ip_current_netif());
        pktInfo->SrcPort     = port;
        pktInfo->DestPort    = pcb->local_port;
    }

Then it's used to get the data elsewhere:

void UDPEndPointImplLwIP::HandleDataReceived(System::PacketBufferHandle && msg)
{
    if ((mState == State::kListening) && (OnMessageReceived != nullptr))
    {
        const IPPacketInfo * pktInfo = GetPacketInfo(msg);

        if (pktInfo != nullptr)
        {
            const IPPacketInfo pktInfoCopy = *pktInfo; // copy the address info so that the app can free the
                                                       // PacketBuffer without affecting access to address info.
            OnMessageReceived(this, std::move(msg), &pktInfoCopy);

The re-alignment logic makes 2 bad assumptions that no longer hold at this point in Matter

1- That the platform is 32 bits
2- That the size of the struct IPPacketInfo will always fit within the "consumed IP header space" from before, which implies we do not grow IPPacketInfo too much

Proposed Solution

• Need to align to native word boundary, not 32 bits
• Need to make sure that sizeof(IPPacketInfo) is < the minimum set of headers one could receive
• Possibly replace this shoehorning with passing more state between calls.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale issue has been automatically closed. Thank you for your contributions.