Re: [ECOS] TFTP bug

[Mark Salter <msalter at redhat dot com>]

>>>>> Pushparaj  writes:

> Hi,
> Today, I looked into the tftp download problem in a more detail. This is what I inferred from
> the packed view that I captured between the tftp server and the client.
> I use the following command to load a s record.
> load c:\image.s19.
> The target hardware which is running on motorola powerpc 860, sends the first packet over UDP
> on the port 69. The server is listening on this port and it replies back saying that its port
> is 1750 and sends the first 512 chunk.
> After this the board sends the ack for this packet with destination port 1750.
> This continues for few more 512 chunks of memory. And the transaction is fine with the server
> sending the packet 'n' and the redboot sending the ack for packet 'n'.
> Now the server transmits the (n+1) packet and suddenly the board sends the ack for nth packet
> again with destination port 0.

I think I see the problem. My guess is that the (n+1) packet is delayed
from the (n) ACK such that the __udp_recvfrom() times out. __udp_recvfrom
starts out by zeroing the port number and will fill it in with the server
port when a packet is received. If no packet is received, the port number
is left at zero. The tftp client code passes in a pointer to its global
server sockaddr structure, so it gets trampled by a recvfrom timeout.

I think this will fix the problem:

Index: tftp_client.c
===================================================================
RCS file: /home/cvs/ecc/ecc/redboot/current/src/net/tftp_client.c,v
retrieving revision 1.11
diff -u -p -5 -c -r1.11 tftp_client.c
cvs server: conflicting specifications of output style
*** tftp_client.c	2001/07/27 16:41:38	1.11
--- tftp_client.c	2001/09/30 20:05:37
*************** tftp_stream_read(char *buf,
*** 273,282 ****
--- 273,283 ----
  {
      int total_bytes = 0;
      int size, recv_len, data_len;
      struct timeval timeout;
      struct tftphdr *hdr = (struct tftphdr *)tftp_stream.data;
+     struct sockaddr_in tmp_addr;
  
      while (total_bytes < len) {
          // Move any bytes which we've already read/buffered
          if (tftp_stream.avail > 0) {
              size = tftp_stream.avail;
*************** tftp_stream_read(char *buf,
*** 303,322 ****
                  break;
              }
              timeout.tv_sec = TFTP_TIMEOUT_PERIOD;
              timeout.tv_usec = 0;
              recv_len = sizeof(tftp_stream.data);
!             if ((data_len = __udp_recvfrom(&tftp_stream.data[0], recv_len, &tftp_stream.from_addr, 
                                             &tftp_stream.local_addr,  &timeout)) < 0) {
                  // No data, try again
                  if ((++tftp_stream.total_timeouts > TFTP_TIMEOUT_MAX) || 
                      (tftp_stream.last_good_block == 0)) {
                      // Timeout - no data received
                      *err = TFTP_TIMEOUT;
                      return -1;
                  }
              } else {
                  if (ntohs(hdr->th_opcode) == DATA) {
                      if (ntohs(hdr->th_block) == (tftp_stream.last_good_block+1)) {
                          // Consume this data
                          data_len -= 4;  /* Sizeof TFTP header */
                          tftp_stream.avail = tftp_stream.actual_len = data_len;
--- 304,325 ----
                  break;
              }
              timeout.tv_sec = TFTP_TIMEOUT_PERIOD;
              timeout.tv_usec = 0;
              recv_len = sizeof(tftp_stream.data);
!             tmp_addr = tftp_stream.from_addr;
!             if ((data_len = __udp_recvfrom(&tftp_stream.data[0], recv_len, &tmp_addr, 
                                             &tftp_stream.local_addr,  &timeout)) < 0) {
                  // No data, try again
                  if ((++tftp_stream.total_timeouts > TFTP_TIMEOUT_MAX) || 
                      (tftp_stream.last_good_block == 0)) {
                      // Timeout - no data received
                      *err = TFTP_TIMEOUT;
                      return -1;
                  }
              } else {
+                 tftp_stream.from_addr = tmp_addr;
                  if (ntohs(hdr->th_opcode) == DATA) {
                      if (ntohs(hdr->th_block) == (tftp_stream.last_good_block+1)) {
                          // Consume this data
                          data_len -= 4;  /* Sizeof TFTP header */
                          tftp_stream.avail = tftp_stream.actual_len = data_len;


--Mark