Merge pull request #11773 from miri64/gnrc_tftp/fix/uninitialized-values

gnrc_tftp: initialize unititialized 'tftp_context_t'
RIOT-OS · Jul 3, 2019 · e8650f5 · e8650f5
2 parents 1d7b825 + 7ba928f
commit e8650f5
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 5 deletions.
diff --git a/sys/include/net/gnrc/tftp.h b/sys/include/net/gnrc/tftp.h
@@ -15,6 +15,9 @@
  * @file
  * @brief       TFTP support library
  *
+ * @warning Currently only runs with link-local addresses when there is only
+ *          one interface
+ *
  * The TFTP module add's support for the TFTP protocol.
  * It implements the following RFC's:
  *  - https://tools.ietf.org/html/rfc1350
@@ -162,6 +165,8 @@ int gnrc_tftp_server_stop(void);
 /**
  * @brief Start an TFTP client read action from the given destination
  *
+ * @pre `(GNRC_NETIF_NUMOF == 1) || !ipv6_addr_is_link_local(addr)`
+ *
  * @param [in] addr         the address of the server
  * @param [in] file_name    the filename of the file to get
  * @param [in] mode         the transfer mode
@@ -180,6 +185,8 @@ int gnrc_tftp_client_read(ipv6_addr_t *addr, const char *file_name, tftp_mode_t
 /**
  * @brief Start an TFTP client write action to the given destination
  *
+ * @pre `(GNRC_NETIF_NUMOF == 1) || !ipv6_addr_is_link_local(addr)`
+ *
  * @param [in] addr         the address of the server
  * @param [in] file_name    the filename of the file to write
  * @param [in] mode         the transfer mode

diff --git a/sys/net/gnrc/application_layer/tftp/gnrc_tftp.c b/sys/net/gnrc/application_layer/tftp/gnrc_tftp.c
@@ -36,6 +36,12 @@
 #include <inttypes.h>
 #endif
 
+#if (GNRC_NETIF_NUMOF > 1)
+/* TODO: change API to make link-local address communitcation with
+ * multiple network interfaces */
+#warning "gnrc_tftp does not work reliably with link-local addresses and >1 network interfaces."
+#endif
+
 static kernel_pid_t _tftp_kernel_pid;
 
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
@@ -405,11 +411,13 @@ int gnrc_tftp_server(tftp_data_cb_t data_cb, tftp_start_cb_t start_cb, tftp_stop
     }
 
     /* context will be initialized when a connection is established */
-    tftp_context_t ctxt;
-    ctxt.data_cb = data_cb;
-    ctxt.start_cb = start_cb;
-    ctxt.stop_cb = stop_cb;
-    ctxt.enable_options = use_options;
+    tftp_context_t ctxt = {
+        .src_port = GNRC_TFTP_DEFAULT_DST_PORT,
+        .data_cb = data_cb,
+        .start_cb = start_cb,
+        .stop_cb = stop_cb,
+        .enable_options = use_options,
+    };
 
     /* validate our arguments */
     assert(data_cb);
@@ -700,6 +708,8 @@ tftp_state _tftp_state_processes(tftp_context_t *ctxt, msg_t *m)
 
             if (proc == TS_DUP) {
                 DEBUG("tftp: duplicated data received, acking...\n");
+                ctxt->dst_port = byteorder_ntohs(udp->src_port);
+                DEBUG("tftp: client's port is %" PRIu16 "\n", ctxt->dst_port);
                 _tftp_send_dack(ctxt, outbuf, TO_ACK);
                 return TS_BUSY;
             }
@@ -991,6 +1001,22 @@ tftp_state _tftp_send(gnrc_pktsnip_t *buf, tftp_context_t *ctxt, size_t len)
         return TS_FAILED;
     }
 
+    if (ipv6_addr_is_link_local(&(ctxt->peer))) {
+        gnrc_pktsnip_t *netif_hdr = gnrc_netif_hdr_build(NULL, 0, NULL, 0);
+        if (netif_hdr == NULL) {
+            DEBUG("tftp: error unable to allocate IPv6 header\n");
+            gnrc_pktbuf_release(ip);
+
+            if (ctxt->stop_cb) {
+                ctxt->stop_cb(TFTP_INTERN_ERROR, "no netif_hdr allocate");
+            }
+
+            return TS_FAILED;
+        }
+        ((gnrc_netif_hdr_t *)netif_hdr->data)->if_pid = gnrc_netif_iter(NULL)->pid;
+        LL_PREPEND(ip, netif_hdr);
+    }
+
     /* send packet */
     if (gnrc_netapi_dispatch_send(GNRC_NETTYPE_UDP, GNRC_NETREG_DEMUX_CTX_ALL,
                                   ip) == 0) {