You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2016/03/07 20:22:15 UTC

trafficserver git commit: TS-4161: fixed ProcessManager stackoverflow vulnerability

Repository: trafficserver
Updated Branches:
  refs/heads/master 0faf94c13 -> d70757aca


TS-4161: fixed ProcessManager stackoverflow vulnerability

This closes #496


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d70757ac
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d70757ac
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d70757ac

Branch: refs/heads/master
Commit: d70757acafa6ba019c8e41c9cee6640f22ac1b03
Parents: 0faf94c
Author: Gancho Tenev <gt...@gmail.com>
Authored: Tue Feb 23 11:19:06 2016 -0800
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Mon Mar 7 12:20:16 2016 -0700

----------------------------------------------------------------------
 mgmt/ProcessManager.cc | 17 +++++++++++++++--
 mgmt/ProcessManager.h  |  2 ++
 2 files changed, 17 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d70757ac/mgmt/ProcessManager.cc
----------------------------------------------------------------------
diff --git a/mgmt/ProcessManager.cc b/mgmt/ProcessManager.cc
index 0dc2300..91aecca 100644
--- a/mgmt/ProcessManager.cc
+++ b/mgmt/ProcessManager.cc
@@ -233,6 +233,7 @@ ProcessManager::pollLMConnection()
   MgmtMessageHdr *mh_full;
   char *data_raw;
 
+  int count = MAX_MSGS_IN_A_ROW;
   while (1) {
     int num;
 
@@ -240,26 +241,38 @@ ProcessManager::pollLMConnection()
     if (num == 0) { /* Have nothing */
       break;
     } else if (num > 0) { /* We have a message */
-
       if ((res = mgmt_read_pipe(local_manager_sockfd, (char *)&mh_hdr, sizeof(MgmtMessageHdr))) > 0) {
-        mh_full = (MgmtMessageHdr *)alloca(sizeof(MgmtMessageHdr) + mh_hdr.data_len);
+        size_t mh_full_size = sizeof(MgmtMessageHdr) + mh_hdr.data_len;
+        mh_full = (MgmtMessageHdr *)ats_malloc(mh_full_size);
+
         memcpy(mh_full, &mh_hdr, sizeof(MgmtMessageHdr));
         data_raw = (char *)mh_full + sizeof(MgmtMessageHdr);
+
         if ((res = mgmt_read_pipe(local_manager_sockfd, data_raw, mh_hdr.data_len)) > 0) {
           Debug("pmgmt", "[ProcessManager::pollLMConnection] Message: '%d'", mh_full->msg_id);
           handleMgmtMsgFromLM(mh_full);
         } else if (res < 0) {
           mgmt_fatal(stderr, errno, "[ProcessManager::pollLMConnection] Error in read!");
         }
+
+        ats_free(mh_full);
       } else if (res < 0) {
         mgmt_fatal(stderr, errno, "[ProcessManager::pollLMConnection] Error in read!");
       }
+
       // handle EOF
       if (res == 0) {
         close_socket(local_manager_sockfd);
         mgmt_fatal(stderr, 0, "[ProcessManager::pollLMConnection] Lost Manager EOF!");
       }
 
+      // Now don't get stuck in the while loop handling too many requests in a row.
+      count--;
+      if (0 == count) {
+        Debug("pmgmt", "[ProcessManager::pollLMConnection] enqueued '%d' messages in a row, pausing for processing",
+              MAX_MSGS_IN_A_ROW);
+        break;
+      }
     } else if (num < 0) { /* Error */
       mgmt_elog(stderr, 0, "[ProcessManager::pollLMConnection] select failed or was interrupted (%d)\n", errno);
     }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d70757ac/mgmt/ProcessManager.h
----------------------------------------------------------------------
diff --git a/mgmt/ProcessManager.h b/mgmt/ProcessManager.h
index 90aa03b..963cd8d 100644
--- a/mgmt/ProcessManager.h
+++ b/mgmt/ProcessManager.h
@@ -100,6 +100,8 @@ public:
   int local_manager_sockfd;
 
 private:
+  static const int MAX_MSGS_IN_A_ROW = 10000;
+
   ConfigUpdateCbTable *cbtable;
 }; /* End class ProcessManager */