[kudu] 01/03: [client] prohibit copying/assigning of ResourceMetrics

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 50395ff74cb47e9ac126e4bca38de1d7e48511c3
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Mon May 9 18:50:26 2022 -0700

    [client] prohibit copying/assigning of ResourceMetrics
    
    Since ResourceMetrics contains a raw pointer, it makes it dangerous
    since its auto-generated copy constructor and assignment operator
    are not private.  Somebody could write something like
    
      {
        KuduScanner scanner();
        {
          ResouceMetrics m = session->GetResourceMetrics();
          ...
        }
        // Continue scanning activity with the 'scanner'.
        ...
      }
    
      or
    
      {
        KuduSession session(...);
        {
          ResouceMetrics m = session->GetWriteOpMetrics();
          ...
        }
        // Continue writing activity with the 'session'.
        ...
      }
    
    and hit a silent memory corruption issue since that's the use-after-free
    condition for the KuduScanner and KuduSession instances when the data
    behind the raw pointer in ResourceMetrics::data_ is accessed later on.
    
    This patch breaks the ABI compatibility for the kudu_client C++ library,
    but this would surface only if there is a dangerous code like above
    in the Kudu application linked with the library.  It's much worse having
    such a hidden memory corruption issue in an application than hitting
    a build breakage due to a linkage error which is quite simple to fix.
    
    This is a follow-up to ece7b5653998db318e4baa5d57f27ba3a836731d.
    
    Change-Id: I602cc4e194a975752687d13d525e44043955a5cf
    Reviewed-on: http://gerrit.cloudera.org:8080/18510
    Tested-by: Alexey Serbin <al...@apache.org>
    Reviewed-by: Riza Suminto <ri...@cloudera.com>
    Reviewed-by: Attila Bukor <ab...@apache.org>
---
 src/kudu/client/resource_metrics.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/kudu/client/resource_metrics.h b/src/kudu/client/resource_metrics.h
index dff31fedc..1b57dada3 100644
--- a/src/kudu/client/resource_metrics.h
+++ b/src/kudu/client/resource_metrics.h
@@ -17,8 +17,9 @@
 #ifndef KUDU_CLIENT_RESOURCE_METRICS_H
 #define KUDU_CLIENT_RESOURCE_METRICS_H
 
-// NOTE: using stdint.h instead of cstdint because this file is supposed
-//       to be processed by a compiler lacking C++11 support.
+// NOTE: using stdint.h instead of cstdint because this file might be
+//       processed by a compiler lacking C++11 support (Kudu C++ client API
+//       still keeps C++98 compatibility)
 #include <stdint.h>
 
 #include <map>
@@ -27,6 +28,7 @@
 #include "kudu/util/kudu_export.h"
 
 #ifdef KUDU_HEADERS_NO_STUBS
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #else
 #include "kudu/client/stubs.h"
@@ -68,6 +70,8 @@ class KUDU_EXPORT ResourceMetrics {
   friend class KuduSession;
   class KUDU_NO_EXPORT Data;
   Data* data_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceMetrics);
 };
 
 } // namespace client