From e2a852699037326c2c8385d98f1b92cd67bf5cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Vavru=C5=A1a?= Date: Wed, 18 Oct 2017 15:13:42 -0700 Subject: [PATCH 1/2] ZooKeeper: fixed stack smashing with tryGet() The tryGet() operation creates a 1MB buffer on stack. This may or may not work depending on the default stack size for threads, whether the stack protector is enabled or not, recursion depth, and the actual value size. This is probably going to slow down some ZK operations, but I don't see how else this could work reliably with the existing API. --- dbms/src/Common/ZooKeeper/ZooKeeper.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dbms/src/Common/ZooKeeper/ZooKeeper.cpp b/dbms/src/Common/ZooKeeper/ZooKeeper.cpp index 765bf494580..9b8b0866928 100644 --- a/dbms/src/Common/ZooKeeper/ZooKeeper.cpp +++ b/dbms/src/Common/ZooKeeper/ZooKeeper.cpp @@ -454,14 +454,16 @@ bool ZooKeeper::existsWatch(const std::string & path, Stat * stat_, const WatchC int32_t ZooKeeper::getImpl(const std::string & path, std::string & res, Stat * stat_, WatchCallback watch_callback) { - char buffer[MAX_NODE_SIZE]; + std::vector buffer; + buffer.reserve(MAX_NODE_SIZE); int buffer_len = MAX_NODE_SIZE; + int32_t code; Stat stat; watcher_fn watcher = watch_callback ? processCallback : nullptr; WatchContext * context = createContext(std::move(watch_callback)); - code = zoo_wget(impl, path.c_str(), watcher, context, buffer, &buffer_len, &stat); + code = zoo_wget(impl, path.c_str(), watcher, context, buffer.data(), &buffer_len, &stat); ProfileEvents::increment(ProfileEvents::ZooKeeperGet); ProfileEvents::increment(ProfileEvents::ZooKeeperTransactions); @@ -473,7 +475,7 @@ int32_t ZooKeeper::getImpl(const std::string & path, std::string & res, Stat * s if (buffer_len < 0) /// This can happen if the node contains NULL. Do not distinguish it from the empty string. res.clear(); else - res.assign(buffer, buffer_len); + res.assign(buffer.data(), buffer_len); } else { From 4f86ec9d587c5d6b58491012d5c6957ab298f553 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Mon, 23 Oct 2017 17:39:29 +0300 Subject: [PATCH 2/2] Update ZooKeeper.cpp --- dbms/src/Common/ZooKeeper/ZooKeeper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Common/ZooKeeper/ZooKeeper.cpp b/dbms/src/Common/ZooKeeper/ZooKeeper.cpp index 9b8b0866928..82491e19bc0 100644 --- a/dbms/src/Common/ZooKeeper/ZooKeeper.cpp +++ b/dbms/src/Common/ZooKeeper/ZooKeeper.cpp @@ -455,7 +455,7 @@ bool ZooKeeper::existsWatch(const std::string & path, Stat * stat_, const WatchC int32_t ZooKeeper::getImpl(const std::string & path, std::string & res, Stat * stat_, WatchCallback watch_callback) { std::vector buffer; - buffer.reserve(MAX_NODE_SIZE); + buffer.resize(MAX_NODE_SIZE); int buffer_len = MAX_NODE_SIZE; int32_t code;