[impala] 09/12: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code

[ta...@apache.org]

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

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 315bc66bbac8715302d455d2d746981cebf74aec
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Mon Mar 12 16:24:35 2018 -0700

    KUDU-2305: Limit sidecars to INT_MAX and fortify socket code
    
    NOTE: This commit is part of a set of changes for IMPALA-7006. It
    contains pieces of a previous commit that need to be cherry picked
    again after rebasing the code in be/src/kudu/{util,security,rpc}.
    
    The original commit message is below:
    
    Inspection of the code revealed some other local variables
    that could overflow with large messages. This patch takes
    two approaches to eliminate the issues.
    
    First, it limits the total size of the messages by limiting
    the total size of the sidecars to INT_MAX. The total size
    of the protobuf and header components of the message
    should be considerably smaller, so limiting the sidecars
    to INT_MAX eliminates messages that are larger than UINT_MAX.
    This also means that the sidecar offsets, which are unsigned
    32-bit integers, are also safe. Given that
    FLAGS_rpc_max_message_size is limited to INT_MAX at startup,
    the receiver would reject any message this large anyway.
    This also helps with the networking codepath, as any given
    sidecar will have a size less than INT_MAX, so every Slice
    that interacts with Writev() is shorter than INT_MAX.
    
    Second, even with sidecars limited to INT_MAX, the headers
    and protobuf parts of the messages mean that certain messages
    could still exceed INT_MAX. This patch changes some of the sockets
    codepath to tolerate iovec's that reference more than INT_MAX
    bytes total. Specifically, it changes Writev()'s nwritten bytes
    to an int64_t for both TlsSocket and Socket. TlsSocket works
    because it is sending each Slice individually. The first change
    limited any given Slice to INT_MAX, so each individual Write()
    should not be impacted. For Socket, Writev() uses sendmsg(). It
    should do partial network sends to handle this case. Any Write()
    call specifies its size with a 32-bit integer, and that will
    not be impacted by this patch.
    
    Testing:
     - Modified TestRpcSidecarLimits() to verify that sidecars are
       limited to INT_MAX bytes.
     - Added a test mode to TestRpcSidecarLimits() where it
       overrides rpc_max_message_size and sends the maximal
       message. This verifies that the client send codepath
       can handle the maximal message.
    
    Reviewed-on: http://gerrit.cloudera.org:8080/9601
    Reviewed-by: Todd Lipcon <to...@apache.org>
    Tested-by: Todd Lipcon <to...@apache.org>
    
    Changes from Kudu version:
     - Updated declaration of FLAGS_rpc_max_message_size
       in rpc-mgr.cc and added a warning not to set it
       larger than INT_MAX.
    
    Change-Id: Id23e518995f2bf2f6bf6b49d5f413f3eaa4e79d1
    Reviewed-on: http://gerrit.cloudera.org:8080/9748
    Reviewed-by: Michael Ho <kw...@cloudera.com>
    Tested-by: Impala Public Jenkins
    Reviewed-on: http://gerrit.cloudera.org:8080/10765
    Reviewed-by: Lars Volker <lv...@cloudera.com>
    Tested-by: Lars Volker <lv...@cloudera.com>
---
 be/src/kudu/rpc/transfer.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/be/src/kudu/rpc/transfer.cc b/be/src/kudu/rpc/transfer.cc
index bdf5191..fefa86e 100644
--- a/be/src/kudu/rpc/transfer.cc
+++ b/be/src/kudu/rpc/transfer.cc
@@ -36,7 +36,7 @@
 #include "kudu/util/logging.h"
 #include "kudu/util/net/socket.h"
 
-DEFINE_int64(rpc_max_message_size, (50 * 1024 * 1024),
+DEFINE_int64_hidden(rpc_max_message_size, (50 * 1024 * 1024),
              "The maximum size of a message that any RPC that the server will accept. "
              "Must be at least 1MB.");
 TAG_FLAG(rpc_max_message_size, advanced);