Don't choke on duplicate OACK packets.
authorMichael Brown <mcb30@etherboot.org>
Wed, 1 Jun 2005 18:00:01 +0000 (18:00 +0000)
committerMichael Brown <mcb30@etherboot.org>
Wed, 1 Jun 2005 18:00:01 +0000 (18:00 +0000)
Make await_tftp() static and create tftp_get() for fetching the next TFTP
packet instead.

src/include/tftpcore.h
src/proto/tftp.c
src/proto/tftpcore.c

index cbe86bd..c0fbd27 100644 (file)
@@ -1,11 +1,19 @@
 #ifndef TFTPCORE_H
 #define TFTPCORE_H
 
+/** @file
+ *
+ * TFTP core functions
+ *
+ * This file provides functions that are common to the TFTP (rfc1350),
+ * TFTM (rfc2090) and MTFTP (PXE) protocols.
+ *
+ */
+
 #include "tftp.h"
 
-extern int await_tftp ( int ival, void *ptr, unsigned short ptype,
-                       struct iphdr *ip, struct udphdr *udp,
-                       struct tcphdr *tcp );
+extern int tftp_get ( struct tftp_state *state, long timeout,
+                     union tftp_any **reply );
 
 extern int tftp_open ( struct tftp_state *state, const char *filename,
                       union tftp_any **reply );
index 5370644..9231c51 100644 (file)
@@ -103,40 +103,47 @@ static int tftp ( char *url __unused, struct sockaddr_in *server, char *file,
                return 0;
        }
        
-       /* Process OACK, if any */
-       if ( ntohs ( reply->common.opcode ) == TFTP_OACK ) {
-               if ( ! tftp_process_opts ( &state, &reply->oack ) ) {
-                       DBG ( "TFTP: option processing failed : %m\n" );
-                       return 0;
-               }
-               reply = NULL;
-       }
-
        /* Fetch file, a block at a time */
        do {
-               /* Get next block to process.  (On the first time
-                * through, we may already have a block from
-                * tftp_open()).
-                */
-               if ( ! reply ) {
-                       if ( ! tftp_ack ( &state, &reply ) ) {
-                               DBG ( "TFTP: could not get next block: %m\n" );
+               twiddle();
+               switch ( ntohs ( reply->common.opcode ) ) {
+               case TFTP_DATA:
+                       if ( ! process_tftp_data ( &state, &reply->data,
+                                                  buffer, &eof ) ) {
+                               tftp_error ( &state, TFTP_ERR_ILLEGAL_OP,
+                                            NULL );
                                return 0;
                        }
-               }
-               twiddle();
-               /* Check it's a DATA block */
-               if ( ntohs ( reply->common.opcode ) != TFTP_DATA ) {
+                       break;
+               case TFTP_OACK:
+                       if ( state.block ) {
+                               /* OACK must be first block, if present */
+                               DBG ( "TFTP: OACK after block %d\n",
+                                     state.block );
+                               errno = PXENV_STATUS_TFTP_UNKNOWN_OPCODE;
+                               tftp_error ( &state, TFTP_ERR_ILLEGAL_OP,
+                                            NULL ); 
+                               return 0;
+                       }
+                       if ( ! tftp_process_opts ( &state, &reply->oack ) ) {
+                               DBG ( "TFTP: option processing failed: %m\n" );
+                               tftp_error ( &state, TFTP_ERR_BAD_OPTS, NULL );
+                               return 0;
+                       }
+                       break;
+               default:
                        DBG ( "TFTP: unexpected opcode %d\n",
                              ntohs ( reply->common.opcode ) );
                        errno = PXENV_STATUS_TFTP_UNKNOWN_OPCODE;
+                       tftp_error ( &state, TFTP_ERR_ILLEGAL_OP, NULL );
                        return 0;
                }
-               /* Process the DATA block */
-               if ( ! process_tftp_data ( &state, &reply->data, buffer,
-                                          &eof ) )
+               /* Fetch the next data block */
+               if ( ! tftp_ack ( &state, &reply ) ) {
+                       DBG ( "TFTP: could not get next block: %m\n" );
+                       tftp_error ( &state, TFTP_ERR_ILLEGAL_OP, NULL );
                        return 0;
-               reply = NULL;
+               }
        } while ( ! eof );
 
        /* ACK the final packet, as a courtesy to the server */
index 491bab6..b97cd68 100644 (file)
@@ -4,17 +4,10 @@
 #include "etherboot.h"
 #include "tftpcore.h"
 
-/** @file
- *
- * TFTP core functions
- *
- * This file provides functions that are common to the TFTP (rfc1350),
- * TFTM (rfc2090) and MTFTP (PXE) protocols.
- *
- */
+/** @file */
 
 /**
- * Wait for a TFTP packet
+ * await_reply() filter for TFTP packets
  *
  * @v ptr                      Pointer to a struct tftp_state
  * @v tftp_state::server::sin_addr TFTP server IP address
@@ -30,7 +23,7 @@
  * and is addressed either to our IP address or to our multicast
  * listening address).
  *
- * Invoke await_tftp() using code such as
+ * Use await_tftp() in code such as
  *
  * @code
  *
@@ -40,9 +33,9 @@
  *
  * @endcode
  */
-int await_tftp ( int ival __unused, void *ptr, unsigned short ptype __unused,
-                struct iphdr *ip, struct udphdr *udp,
-                struct tcphdr *tcp __unused ) {
+static int await_tftp ( int ival __unused, void *ptr,
+                       unsigned short ptype __unused, struct iphdr *ip,
+                       struct udphdr *udp, struct tcphdr *tcp __unused ) {
        struct tftp_state *state = ptr;
 
        /* Must have valid UDP (and, therefore, also IP) headers */
@@ -76,6 +69,51 @@ int await_tftp ( int ival __unused, void *ptr, unsigned short ptype __unused,
        return 1;
 }
 
+/**
+ * Retrieve a TFTP packet
+ *
+ * @v state                    TFTP transfer state
+ * @v tftp_state::server::sin_addr TFTP server IP address
+ * @v tftp_state::client::sin_addr Client multicast IP address, or 0.0.0.0
+ * @v tftp_state::client::sin_port Client UDP port
+ * @v timeout                  Time to wait for a response
+ * @ret True                   Received a non-error response
+ * @ret False                  Received error response / no response
+ * @ret *reply                 The server's response, if any
+ * @err #PXENV_STATUS_TFTP_READ_TIMEOUT No response received in time
+ * @err other                  As set by tftp_set_errno()
+ *
+ * Retrieve the next packet sent by the TFTP server, if any is sent
+ * within the specified timeout period.  The packet is returned via
+ * #reply.  If no packet is received within the timeout period, a NULL
+ * value will be stored in #reply.
+ *
+ * If the response from the server is a TFTP ERROR packet, tftp_get()
+ * will return False and #errno will be set accordingly.
+ *
+ * You can differentiate between "received no response" and "received
+ * an error response" by checking #reply; if #reply is NULL then no
+ * response was received.
+ */
+int tftp_get ( struct tftp_state *state, long timeout,
+              union tftp_any **reply ) {
+
+       *reply = NULL;
+
+       if ( ! await_reply ( await_tftp, 0, state, timeout ) ) {
+               errno = PXENV_STATUS_TFTP_READ_TIMEOUT;
+               return 0;
+       }
+
+       *reply = ( union tftp_any * ) &nic.packet[ETH_HLEN];
+       DBG ( "TFTPCORE: got reply (type %d)\n",
+             ntohs ( (*reply)->common.opcode ) );
+       if ( ntohs ( (*reply)->common.opcode ) == TFTP_ERROR ){
+               tftp_set_errno ( &(*reply)->error );
+               return 0;
+       }
+       return 1;
+}
 
 /**
  * Issue a TFTP open request (RRQ)
@@ -210,20 +248,19 @@ int tftp_open ( struct tftp_state *state, const char *filename,
                        return 0;
                
                /* Wait for response */
-               if ( await_reply ( await_tftp, 0, state, timeout ) ) {
-                       *reply = ( union tftp_any * ) &nic.packet[ETH_HLEN];
-                       state->server.sin_port =
-                               ntohs ( (*reply)->common.udp.src );
-                       DBG ( "TFTPCORE: got reply from %@:%d (type %d)\n",
+               if ( tftp_get ( state, timeout, reply ) ) {
+                       /* We got a non-error response */
+                       state->server.sin_port
+                               ntohs ( (*reply)->common.udp.src );
+                       DBG ( "TFTP server is at %@:%d\n",
                              state->server.sin_addr.s_addr,
-                             state->server.sin_port,
-                             ntohs ( (*reply)->common.opcode ) );
-                       if ( ntohs ( (*reply)->common.opcode ) == TFTP_ERROR ){
-                               tftp_set_errno ( &(*reply)->error );
-                               return 0;
-                       }
+                             state->server.sin_port );
                        return 1;
                }
+               if ( reply ) {
+                       /* We got an error response; abort */
+                       return 0;
+               }
        }
 
        DBG ( "TFTPCORE: open request timed out\n" );
@@ -421,18 +458,15 @@ int tftp_ack ( struct tftp_state *state, union tftp_any **reply ) {
                if ( ! tftp_ack_nowait ( state ) ) {
                        DBG ( "TFTP: could not send ACK: %m\n" );
                        return 0;
-               }       
-               if ( await_reply ( await_tftp, 0, state, timeout ) ) {
-                       /* We received a reply */
-                       *reply = ( union tftp_any * ) &nic.packet[ETH_HLEN];
-                       DBG ( "TFTPCORE: got reply (type %d)\n",
-                             ntohs ( (*reply)->common.opcode ) );
-                       if ( ntohs ( (*reply)->common.opcode ) == TFTP_ERROR ){
-                               tftp_set_errno ( &(*reply)->error );
-                               return 0;
-                       }
+               }
+               if ( tftp_get ( state, timeout, reply ) ) {
+                       /* We got a non-error response */
                        return 1;
                }
+               if ( reply ) {
+                       /* We got an error response */
+                       return 0;
+               }
        }
        DBG ( "TFTP: timed out during read\n" );
        errno = PXENV_STATUS_TFTP_READ_TIMEOUT;
@@ -447,12 +481,15 @@ int tftp_ack ( struct tftp_state *state, union tftp_any **reply ) {
  * @v tftp_state::server::sin_port     TFTP server UDP port
  * @v tftp_state::client::sin_port     Client UDP port
  * @v errcode                          TFTP error code
- * @v errmsg                           Descriptive error string
+ * @v errmsg                           Descriptive error string, or NULL
  * @ret True                           Error packet was sent
  * @ret False                          Error packet was not sent
  *
  * Send a TFTP ERROR packet back to the server to terminate the
  * transfer.
+ *
+ * If #errmsg is NULL, the current error message string as returned by
+ * strerror(errno) will be used as the error text.
  */
 int tftp_error ( struct tftp_state *state, int errcode, const char *errmsg ) {
        struct tftp_error error;
@@ -460,7 +497,8 @@ int tftp_error ( struct tftp_state *state, int errcode, const char *errmsg ) {
        DBG ( "TFTPCORE: aborting with error %d (%s)\n", errcode, errmsg );
        error.opcode = htons ( TFTP_ERROR );
        error.errcode = htons ( errcode );
-       strncpy ( error.errmsg, errmsg, sizeof ( error.errmsg ) );
+       strncpy ( error.errmsg, errmsg ? errmsg : strerror ( errno ),
+                 sizeof ( error.errmsg ) );
        return udp_transmit ( state->server.sin_addr.s_addr,
                              state->client.sin_port, state->server.sin_port,
                              sizeof ( error ), &error );