UDP receive bug

[eugeneryabtsev]

In MQX 4.2 in rtcs\source\tcpip\udp.c I see the following:

static void udp_return_req2socket_from_rx_queue(UDP_PARM_PTR parms, struct udp_rx_dgram_header * dgram_item)
{
  uint32_t dgram_size = 0; 
  dgram_size = dgram_item->size; 
  /* limit for the upper layer recv buffer */ 
  if(parms->udpword > dgram_size)
  {
    parms->udpword = dgram_size;
  }
  if(parms->saddr_ptr)
  {
    *parms->saddr_ptr = dgram_item->fromaddr;
  }
  memcpy(parms->udpptr, dgram_item->dgram, dgram_size);
  ((SOCKET_STRUCT_PTR)parms->ucb->SOCKET)->LINK_OPTIONS.RX = dgram_item->rx_linkopts;
}

I'd rather expect it to be more like this:

  ...
  memcpy(parms->udpptr, dgram_item->dgram, parms->udpword /* dgram_size */);
  ...

This seems to respectively cause and solve a system-crash-sized-problem in our application.

[avm]

This is an old thread, but I have run into the same problem. I found this thread by searching for the name of the function udp_return_req2socket_from_rx_queue()

In my case, I'm using the MQX 4.2 TFTP client, and not writing my own socket handling code. The remote TFTP server on the other end of the transfer is also running MQX 4.2. I'm able to "get" files successfully, but the "TFTPCLN task" consistently crashes when trying to "put" a file to the remote system.

I have tracked it down to this function. The TFTP client task sends a request to start the transfer, and then calls tftp_recv_ack() to receive an acknowledgment packet. It passes in a pointer to a 4-byte packet, and an expected length of 4. This read request passes through the UDP and TCP stacks, and eventually ends up inside udp_return_req2socket_from_rx_queue() where it is processing a 25 byte datagram that it has received. It crams the 25 bytes from the datagram into the 4 byte buffer, blowing away the data that is allocated after that buffer and corrupting memory.

Things are still running, and eventually the TFTP client tries to send the first data packet. To do this, it starts building a datagram by poking the TFTP_OPCODE_DATA value into the transmit buffer. But the pointer to that buffer is part of the data that got clobbered by the previous receive data overrun, so that pointer address is no longer valid and processor generates a hard fault. The system is now dead.

I find it curious that that the length of the data buffer to receive a datagram is passed into udp_return_req2socket_from_rx_queue(), but the function does not honor it. Instead, it writes the whole datagram into the receive buffer and clobbers memory.

I have tried Eugene Ryabtsev's change, and it seems to work. Hopefully this isn't introducing other issues.

[danielchen]

Hi

I think in normal cases, parms->udpword should be great  than  dgram_size. It seems that in your specific case, parms->udpword < dgram_size.  not sure why. need more information to analyze.

But if you modify the code to the following

... 

  memcpy(parms->udpptr, dgram_item->dgram, parms->udpword /* dgram_size */); 

  ... 

maybe in some cases, you can't copy  the whole source data to the destination.

Regards

Daniel

[eugeneryabtsev]

We go something like this:

        byte buffer[100];
        while (true) {
            sockaddr_in peerAddr;
            uint16_t peerAddrLen = sizeof peerAddr;
            uint32_t len = recvfrom(sock, buffer, sizeof buffer, 0, (sockaddr *) &peerAddr, &peerAddrLen);
            ...
        }

The datagrams aimed at us are both under 100 bytes (of interest to us) and over 100 bytes (of no interest to us). The problem seem to be triggered by the reception of a longer datagram.