0 Replies Latest reply on May 28, 2017 7:28 AM by tuankiet65

    Bug in corelibs-edison at WiFiClient.stop() (use of uninitialized pointer)

    tuankiet65

      Hi all,

       

      I'm developing on the Intel Edison usning the Arduino IDE. My sketch uses the WiFi library. I notice that if my sketch calls WiFiClient::stop() then it will randomly crashes without any explanation. The reason I need WiFiClient::stop() is because my sketch needs to repeatedly query a specific URL every 1 second, and if I don't call stop() on existing clients then I'll run out of sockets to use.

       

      To investigate the culprit I run Valgrind on my sketch and it says (Highlighted texts are the important bits):

      ==6239== Use of uninitialised value of size 4
      ==6239==    at 0x8058C73: WiFiClient::stop() (WiFiClient.cpp:272)
      ==6239==    by 0x804BEF8: HTTP::Client::post(std::string, short, std::string, std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > >) (http_client.cpp:32)
      ==6239==    by 0x804D3A7: SmartBotsAPI::get_pending_cmds(std::vector<send_msg, std::allocator<send_msg> >&) (smartbots_api.cpp:81)
      ==6239==    by 0x805082F: loop() (smartbots_hub.ino:45)
      ==6239==    by 0x805F35D: main (main.cpp:114)
      ==6239==  Uninitialised value was created by a stack allocation
      ==6239==    at 0x804D256: SmartBotsAPI::get_pending_cmds(std::vector<send_msg, std::allocator<send_msg> >&) (smartbots_api.cpp:68)
      ==6239== 
      ==6239== Invalid write of size 4
      ==6239==    at 0x8058C73: WiFiClient::stop() (WiFiClient.cpp:272)
      ==6239==    by 0x804BEF8: HTTP::Client::post(std::string, short, std::string, std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > >) (http_client.cpp:32)
      ==6239==    by 0x804D3A7: SmartBotsAPI::get_pending_cmds(std::vector<send_msg, std::allocator<send_msg> >&) (smartbots_api.cpp:81)
      ==6239==    by 0x805082F: loop() (smartbots_hub.ino:45)
      ==6239==    by 0x805F35D: main (main.cpp:114)
      ==6239==  Address 0x2 is not stack'd, malloc'd or (recently) free'd
      ==6239== 
      ==6239== 
      ==6239== Process terminating with default action of signal 11 (SIGSEGV)
      ==6239==  Access not within mapped region at address 0x2
      ==6239==    at 0x8058C73: WiFiClient::stop() (WiFiClient.cpp:272)
      ==6239==    by 0x804BEF8: HTTP::Client::post(std::string, short, std::string, std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > >) (http_client.cpp:32)
      ==6239==    by 0x804D3A7: SmartBotsAPI::get_pending_cmds(std::vector<send_msg, std::allocator<send_msg> >&) (smartbots_api.cpp:81)
      ==6239==    by 0x805082F: loop() (smartbots_hub.ino:45)
      ==6239==    by 0x805F35D: main (main.cpp:114)
      

      (loop, SmartBotsAPI, HTTP are my functions/classes/namespaces)

       

      Looking at WiFiClient.cpp at around the line 272 I found the following (GitHub link):

      ...
      if (_inactive_counter != NULL) // Line 271
           *_inactive_counter = 0;   // Line 272
      

       

      And in WiFiClient.h _inactive_counter is defined as follow (GitHub link):

      int *_inactive_counter;
      

       

      _inactive_counter is a pointer to an int variable, however it isn't initialized to any variables. Thus, it's a wild pointer pointing to an undefined address in memory. And of course when dereferencing/writing a value into a wild pointer, there'll be a very high chance that the address is invalid, and the program will terminated because of segmentation fault.

       

      Looking further, there's only one place where _inactive_counter is assigned to a variable. It's in WiFiServer.cpp at line 131 (Github link)

      pclients[sock]._inactive_counter = &_pcli_inactivity_counter[sock];
      

       

      The Ethernet library is also affected by this bug.

       

      My workaround currently is to remove any mention of _inactive_counter, however I won't recommend that because there may be real use of it.

       

      Message was edited by: Kiet Ho Reason: Fix valgrind log formatting