Hello Michael Ho, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9601
to look at the new patch set (#3).
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 the
rpc_max_message_size is 32-bit integer, the receiver would
reject any message with sidecars this large anyway. This also
helps with the networking codepath, as any given sidecar will
have a size less than INT_MAX, so all Slices that interact
with Writev() are each less 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.
Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
---
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
M src/kudu/rpc/rpc_sidecar.cc
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
M src/kudu/security/tls_socket-test.cc
M src/kudu/security/tls_socket.cc
M src/kudu/security/tls_socket.h
M src/kudu/util/net/socket.cc
M src/kudu/util/net/socket.h
16 files changed, 80 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/9601/3
--
To view, visit http://gerrit.cloudera.org:8080/9601
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
Gerrit-Change-Number: 9601
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
