Joe McDonnell has posted comments on this change. ( )

Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code

Patch Set 1:

File src/kudu/rpc/inbound_call.h:
PS1, Line 253: outbound_sidecars_size_
> I think 'total_size_' or 'size_bytes_' or something is more clear, since 's
Changed to "outbound_sidecars_total_bytes_"
File src/kudu/rpc/
PS1, Line 186: absolute_sidecar_offset - protobuf_msg_size
> these are both uint32_t, right? maybe we should change absolute_sidecar_off
Good point, best to be defensive. Changed absolute_sidecar_offset to an int64_t 
and added a check for it be less than UINT_MAX.
PS1, Line 221: exceeds limit
> would exceed
File src/kudu/rpc/rpc_controller.h:
PS1, Line 275:   int64_t outbound_sidecars_size_ = 0;
> same comment as on the InboundCall
File src/kudu/rpc/
PS1, Line 153: exceeds limit
> would exceed
File src/kudu/rpc/transfer.h:
PS1, Line 51:     kMaxTotalSidecarSize = INT_MAX
> it seems elsewhere in this patch you are checking against INT_MAX instead o
Yes, my comments are confusing. The checks for sidecars use 
kMaxTotalSidecarBytes. However, several of the checks are to verify that 
arguments don't overflow. For example, SerializeMessage() takes additional_size 
as an int32_t, but the caller is using an int64_t local variable, so it's a bit 
different from the sidecar limits. I do something similar for the sidecar 
offsets, which are limited to uint32_t. I changed the comments to clarify this 
a bit. I also changed to use kMaxTotalSidecarBytes rather than 

To view, visit
To unsubscribe, visit

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24
Gerrit-Change-Number: 9601
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-Comment-Date: Tue, 13 Mar 2018 20:05:45 +0000
Gerrit-HasComments: Yes

Reply via email to