[Bugfix](stack_over_flow) fix be may core dump because of stack-buffe…

…r-overflow when TBrokerOpenReaderResponse too large (apache#12658)
FreeOnePlus · Oct 8, 2022 · c2dee0c · c2dee0c
1 parent b3c483d
commit c2dee0c
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 12 deletions.
diff --git a/be/src/exprs/bitmap_function.cpp b/be/src/exprs/bitmap_function.cpp
@@ -822,10 +822,19 @@ void BitmapFunctions::orthogonal_bitmap_count_merge(FunctionContext* context, co
 // finalize for ORTHOGONAL_BITMAP_UNION_COUNT(bitmap)
 BigIntVal BitmapFunctions::orthogonal_bitmap_count_finalize(FunctionContext* context,
                                                             const StringVal& src) {
-    auto* pval = reinterpret_cast<int64_t*>(src.ptr);
-    int64_t result = *pval;
-    delete pval;
-    return result;
+    if (src.is_null) {
+        return BigIntVal::null();
+    } else if (src.len == sizeof(int64_t)) {
+        auto* pval = reinterpret_cast<int64_t*>(src.ptr);
+        BigIntVal result = BigIntVal(*pval);
+        delete pval;
+        return result;
+    } else {
+        auto src_bitmap = reinterpret_cast<BitmapValue*>(src.ptr);
+        BigIntVal result = BigIntVal(src_bitmap->cardinality());
+        delete src_bitmap;
+        return result;
+    }
 }
 
 // This is a serialize function for orthogonal_bitmap_intersect_count(bitmap,t,t).

diff --git a/be/src/io/broker_reader.cpp b/be/src/io/broker_reader.cpp
@@ -25,6 +25,7 @@
 #include "runtime/broker_mgr.h"
 #include "runtime/client_cache.h"
 #include "runtime/exec_env.h"
+#include "util/defer_op.h"
 
 namespace doris {
 
@@ -76,7 +77,8 @@ Status BrokerReader::open() {
     request.__set_clientId(client_id(_env, broker_addr));
     request.__set_properties(_properties);
 
-    TBrokerOpenReaderResponse response;
+    TBrokerOpenReaderResponse* response = new TBrokerOpenReaderResponse();
+    Defer del_reponse {[&] { delete response; }};
     try {
         Status status;
         BrokerServiceConnection client(client_cache(_env), broker_addr,
@@ -88,11 +90,11 @@ Status BrokerReader::open() {
         }
 
         try {
-            client->openReader(response, request);
+            client->openReader(*response, request);
         } catch (apache::thrift::transport::TTransportException& e) {
             std::this_thread::sleep_for(std::chrono::seconds(1));
             RETURN_IF_ERROR(client.reopen());
-            client->openReader(response, request);
+            client->openReader(*response, request);
         }
     } catch (apache::thrift::TException& e) {
         std::stringstream ss;
@@ -101,21 +103,21 @@ Status BrokerReader::open() {
         return Status::RpcError(ss.str());
     }
 
-    if (response.opStatus.statusCode != TBrokerOperationStatusCode::OK) {
+    if (response->opStatus.statusCode != TBrokerOperationStatusCode::OK) {
         std::stringstream ss;
         ss << "Open broker reader failed, broker:" << broker_addr
-           << " failed:" << response.opStatus.message;
+           << " failed:" << response->opStatus.message;
         LOG(WARNING) << ss.str();
         return Status::InternalError(ss.str());
     }
     // TODO(cmy): The file size is no longer got from openReader() method.
     // But leave the code here for compatibility.
     // This will be removed later.
-    if (response.__isset.size) {
-        _file_size = response.size;
+    if (response->__isset.size) {
+        _file_size = response->size;
     }
 
-    _fd = response.fd;
+    _fd = response->fd;
     _is_fd_valid = true;
     return Status::OK();
 }