Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9748 )
Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code ...................................................................... KUDU-2305: Limit sidecars to INT_MAX and fortify socket code 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 <[email protected]> Tested-by: Todd Lipcon <[email protected]> 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: I469feff940fdd07e1e407c9df49de79ed303151e Reviewed-on: http://gerrit.cloudera.org:8080/9748 Reviewed-by: Michael Ho <[email protected]> Tested-by: Impala Public Jenkins --- M be/src/kudu/rpc/inbound_call.cc M be/src/kudu/rpc/inbound_call.h M be/src/kudu/rpc/outbound_call.cc M be/src/kudu/rpc/outbound_call.h M be/src/kudu/rpc/rpc-test.cc M be/src/kudu/rpc/rpc_controller.cc M be/src/kudu/rpc/rpc_controller.h M be/src/kudu/rpc/rpc_sidecar.cc M be/src/kudu/rpc/serialization.cc M be/src/kudu/rpc/transfer.cc M be/src/kudu/rpc/transfer.h M be/src/kudu/security/tls_socket.cc M be/src/kudu/security/tls_socket.h M be/src/kudu/util/net/socket.cc M be/src/kudu/util/net/socket.h M be/src/rpc/rpc-mgr.cc M security/tls_socket-test.cc 17 files changed, 135 insertions(+), 38 deletions(-) Approvals: Michael Ho: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9748 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I469feff940fdd07e1e407c9df49de79ed303151e Gerrit-Change-Number: 9748 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]>
