HTTPD bug fix which is caused by stack overflow.
Dependents: mbed_controller_demo
Fork of HTTPD by
Original HTTPD implementation of Suga koubou is great but has some bug inside unfortunately. The most critical bug was accessing buffer with the index of out of range like following.
problematic code
char buf[256]; n = httpd->_state[id].client->receive(buf, sizeof(buf)); buf[n] =0;
With above code, it could set buf[256] = 0 when more that 255 data is received. Setting buf[256] causes some other area of memory is corrupted so that system can be predictive status since than.
bug fixed code
n = httpd->_state[id].client->receive(buf, sizeof(buf)-1); buf[n] =0;
Revision 2:584ce0a1a76e, committed 2015-04-10
- Comitter:
- hillkim7
- Date:
- Fri Apr 10 09:04:38 2015 +0000
- Parent:
- 1:a4c4bd58dd3b
- Commit message:
- Fix critical bug cause by accessing buffer with index of out of range.; Set reasonable stack size for server task.
Changed in this revision
diff -r a4c4bd58dd3b -r 584ce0a1a76e HTTPD.cpp --- a/HTTPD.cpp Fri Mar 20 12:08:16 2015 +0000 +++ b/HTTPD.cpp Fri Apr 10 09:04:38 2015 +0000 @@ -18,11 +18,20 @@ #include "HTTPD.h" -// There was Hard Fault error in HTTPD with Seeed Arch Max platform -// because of stack overflow. -// According to the Callgraph report generated by Keil MDK 4.1, -// the Max Depth of HTTPD::daemon() is 424 bytes. +/* +// There was hald fault error of HTTPD with Seeed Arch Max platform +// because of stack overwritten. +// According to the Callgraph report of Keil MDK 4.1, +// the Max Depth is 424 bytes. +HTTPD::daemon(const void*) (Thumb, 110 bytes, Stack size 0 bytes, httpd.o(.text)) +[Stack] + +Max Depth = 424 + Unknown Stack Size +Call Chain = HTTPD::daemon(const void*) <-> TCPSocketServer::accept(TCPSocketConnection&) <-> +Socket::wait_readable(TimeInterval&) <-> Socket::select(timeval*, bool, bool) <-> +lwip_select <-> lwip_selscan <-> sys_arch_unprotect <-> error <-> exit (Cycle) +*/ #define DAEMON_STACK_SIZE 512 HTTPD * HTTPD::_inst; @@ -98,7 +107,7 @@ for (;;) { if (! httpd->_state[id].client->is_connected()) break; - n = httpd->_state[id].client->receive(buf, sizeof(buf)); + n = httpd->_state[id].client->receive(buf, sizeof(buf)-1); if (n < 0) break; buf[n] = 0; // DBG("Recv %d '%s'", n, buf); @@ -124,4 +133,3 @@ INFO("Close %s\r\n", httpd->_state[id].client->get_address()); } } -
diff -r a4c4bd58dd3b -r 584ce0a1a76e HTTPD.h --- a/HTTPD.h Fri Mar 20 12:08:16 2015 +0000 +++ b/HTTPD.h Fri Apr 10 09:04:38 2015 +0000 @@ -34,7 +34,19 @@ #define HTTPD_CMD_SIZE 100 #define HTTPD_BUF_SIZE 256 -#define HTTPD_STACK_SIZE (1024 * 6) +//#define HTTPD_STACK_SIZE (1024 * 6) +// The HTTPD_STACK_SIZE is too much for clild thread, so it is reduced. +// See following Callgraph report of Keil MDK. +#define HTTPD_STACK_SIZE (1024 * 2) +/* +HTTPD::child(const void*) (Thumb, 244 bytes, Stack size 272 bytes, httpd.o(.text)) + +[Stack] + +Max Depth = 1288 + Unknown Stack Size +Call Chain = HTTPD::child(const void*) <-> HTTPD::recvData(int, char) <-> +HTTPD::httpdFile(int, char*) <-> fopen <-> _freopen_locked <-> _sys_open <-> +*/ //#define HTTPD_ENABLE_CLOSER //Debug is disabled by default @@ -169,3 +181,4 @@ }; #endif +
diff -r a4c4bd58dd3b -r 584ce0a1a76e HTTPD_req.cpp --- a/HTTPD_req.cpp Fri Mar 20 12:08:16 2015 +0000 +++ b/HTTPD_req.cpp Fri Apr 10 09:04:38 2015 +0000 @@ -226,7 +226,7 @@ int i, j, len; char buf[HTTPD_CMD_SIZE]; - for (len = 0; len < sizeof(buf); len++) { + for (len = 0; len < sizeof(buf)-1; len++) { if (_state[id].buf->dequeue(&buf[len]) == false) break; } buf[len] = 0; @@ -278,7 +278,7 @@ int i; char buf[HTTPD_CMD_SIZE]; static const struct HEADER_TABLE { - const char header[24]; + const char *header; void (HTTPD::*func)(int id, const char*); } header_table[HEADER_TABLE_NUM] = { {"Content-Length:", &HTTPD::reqContentLength}, @@ -287,7 +287,7 @@ {"Sec-WebSocket-Version:", &HTTPD::reqWebSocketVersion}, {"Sec-WebSocket-Key:", &HTTPD::reqWebSocketKey}, }; - for (i = 0; i < sizeof(buf); i++) { + for (i = 0; i < sizeof(buf)-1; i++) { if (_state[id].buf->dequeue(&buf[i]) == false) break; } buf[i] = 0; @@ -326,8 +326,11 @@ } void HTTPD::reqWebSocketKey (int id, const char *buf) { + const unsigned int max_key_len = 29; if (_state[id].websocket_key == NULL) { - _state[id].websocket_key = (char*)malloc(30); + _state[id].websocket_key = (char*)malloc(max_key_len+1); } - strncpy(_state[id].websocket_key, &buf[19], 30); + strncpy(_state[id].websocket_key, &buf[19], max_key_len); + _state[id].websocket_key[max_key_len] = 0; } +