From 04889813f325695f33facf44fa8ecae6ce7b0560 Mon Sep 17 00:00:00 2001
From: Johny Mattsson <jmattsson@dius.com.au>
Date: Tue, 1 Mar 2016 17:54:10 +1100
Subject: [PATCH] Fix nasty case of memory corruption by enduser_setup.

Turns out that running wifi_station_config() from a network callback can
be a Really Really Bad Idea(tm).
---
 app/modules/enduser_setup.c | 55 +++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 18 deletions(-)

diff --git a/app/modules/enduser_setup.c b/app/modules/enduser_setup.c
index a425d913..f869051a 100644
--- a/app/modules/enduser_setup.c
+++ b/app/modules/enduser_setup.c
@@ -43,6 +43,7 @@
 #include "user_interface.h"
 #include "espconn.h"
 #include "flash_fs.h"
+#include "task/task.h"
 
 #define MIN(x, y)  (((x) < (y)) ? (x) : (y))
 #define LITLEN(strliteral) (sizeof (strliteral) -1)
@@ -244,6 +245,7 @@ typedef struct
 
 static enduser_setup_state_t *state;
 static bool manual = false;
+static task_handle_t do_station_cfg_handle;
 
 static int enduser_setup_start(lua_State* L);
 static int enduser_setup_stop(lua_State* L);
@@ -492,6 +494,32 @@ static void enduser_setup_http_urldecode(char *dst, const char *src, int src_len
 }
 
 
+/**
+ * Task to do the actual station configuration.
+ * This config *cannot* be done in the network receive callback or serious
+ * issues like memory corruption occur.
+ */
+static void do_station_cfg (task_param_t param, uint8_t prio)
+{
+  struct station_config *cnf = (struct station_config *)param;
+  (void)prio;
+
+  /* Best-effort disconnect-reconfig-reconnect. If the device is currently
+   * connected, the disconnect will work but the connect will report failure
+   * (though it will actually start connecting). If the devices is not
+   * connected, the disconnect may fail but the connect will succeed. A
+   * solid head-in-the-sand approach seems to be the best tradeoff on
+   * functionality-vs-code-size.
+   * TODO: maybe use an error callback to at least report if the set config
+   * call fails.
+   */
+  wifi_station_disconnect ();
+  wifi_station_set_config (cnf);
+  wifi_station_connect ();
+  os_free (cnf);
+}
+
+
 /**
  * Handle HTTP Credentials
  *
@@ -522,30 +550,18 @@ static int enduser_setup_http_handle_credentials(char *data, unsigned short data
     return 1;
   }
 
-  struct station_config cnf;
-  c_memset(&cnf, 0, sizeof(struct station_config));
-  enduser_setup_http_urldecode(cnf.ssid, name_str_start, name_str_len);
-  enduser_setup_http_urldecode(cnf.password, pwd_str_start, pwd_str_len);
+  struct station_config *cnf = os_zalloc(sizeof(struct station_config));
+  enduser_setup_http_urldecode(cnf->ssid, name_str_start, name_str_len);
+  enduser_setup_http_urldecode(cnf->password, pwd_str_start, pwd_str_len);
 
-  if (!wifi_station_disconnect())
-  {
-    ENDUSER_SETUP_ERROR("station_start failed. wifi_station_disconnect failed.", ENDUSER_SETUP_ERR_UNKOWN_ERROR, ENDUSER_SETUP_ERR_NONFATAL);
-  }
-  if (!wifi_station_set_config(&cnf))
-  {
-    ENDUSER_SETUP_ERROR("station_start failed. wifi_station_set_config failed.", ENDUSER_SETUP_ERR_UNKOWN_ERROR, ENDUSER_SETUP_ERR_FATAL);
-  }
-  if (wifi_station_connect())
-  {
-    ENDUSER_SETUP_ERROR("station_start failed. wifi_station_connect failed.\n", ENDUSER_SETUP_ERR_UNKOWN_ERROR, ENDUSER_SETUP_ERR_FATAL);
-  }
+  task_post_medium(do_station_cfg_handle, (task_param_t)cnf);
 
   ENDUSER_SETUP_DEBUG(lua_getstate(), "WiFi Credentials Stored");
   ENDUSER_SETUP_DEBUG(lua_getstate(), "-----------------------");
   ENDUSER_SETUP_DEBUG(lua_getstate(), "name: ");
-  ENDUSER_SETUP_DEBUG(lua_getstate(), cnf.ssid);
+  ENDUSER_SETUP_DEBUG(lua_getstate(), cnf->ssid);
   ENDUSER_SETUP_DEBUG(lua_getstate(), "pass: ");
-  ENDUSER_SETUP_DEBUG(lua_getstate(), cnf.password);
+  ENDUSER_SETUP_DEBUG(lua_getstate(), cnf->password);
   ENDUSER_SETUP_DEBUG(lua_getstate(), "-----------------------");
 
   return 0;
@@ -1248,6 +1264,9 @@ static int enduser_setup_start(lua_State *L)
 {
   ENDUSER_SETUP_DEBUG(L, "enduser_setup_start");
 
+  if (!do_station_cfg_handle)
+    do_station_cfg_handle = task_get_id(do_station_cfg);
+
   if(enduser_setup_init(L))
     goto failed;