[riot-notifications] [RIOT-OS/RIOT] gnrc_tftp: initialize unititialized 'tftp_context_t' (#11773)

Martine Lenders notifications at github.com
Wed Jul 3 11:42:52 CEST 2019


miri64 commented on this pull request.



> @@ -405,11 +405,14 @@ 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 = {
+        .dst_port = GNRC_TFTP_DEFAULT_DST_PORT,
+        .src_port = GNRC_TFTP_DEFAULT_DST_PORT,

> Assuming my understanding of the protocol and the implementation here is sufficient, this .src_port is the UDP port used as source by RIOT and used as destination by the peer. Both should be ephermal ports specific for this connection.

We are in server context here, so `src_port` should at least be `GNRC_TFTP_DEFAULT_DST_PORT` (which is the default TFTP port). Ephemeral ports in GNRC are part of the `gnrc_sock` implementation, which this implementation does not use :-/

> Does it make sense to default it to something invalid/unconfigured? (Do we have something like that in gnrc?)

If I leave it unconfigured, the testing procedure in #11772 runs into an assertion here

https://github.com/RIOT-OS/RIOT/blob/a01e70a4ab0050331b36ad59946637c62c48ed3a/sys/net/gnrc/transport_layer/udp/gnrc_udp.c#L283-L286

from the call here

https://github.com/RIOT-OS/RIOT/blob/a01e70a4ab0050331b36ad59946637c62c48ed3a/sys/net/gnrc/application_layer/tftp/gnrc_tftp.c#L967-L969

This is the only application protocol purely based on GNRC (and as soon as we have async sock, I prefer to have it ported to `sock_udp`, but first we need to fix the bug ;-)).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11773#discussion_r299868809
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190703/afe16d6a/attachment.html>


More information about the notifications mailing list