Repository: kudu Updated Branches: refs/heads/master 7ac7762ff -> be62673d9
KUDU-2004. Undefined behavior in TlsSocket::Writev() TlsSocket::Writev() was attempting to use the value of nwritten from TlsSocket::Write(), but in the case of an error that value was never set or initialized. A simple check to make sure the result from TlsSocket::Write() wasn't an error was added, otherwise we break out of the write loop to cleanup and return the error (thus skipping the line that uses nwritten) Dist job result from before the fix: http://dist-test.cloudera.org/job?job_id=efan.1496860112.16151 Dist job result from after the fix: http://dist-test.cloudera.org/job?job_id=efan.1497036430.19311 Change-Id: Ia5b5bbb3fd2ec8fcd1a48873446f3aa09546eaac Reviewed-on: http://gerrit.cloudera.org:8080/7141 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/be62673d Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/be62673d Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/be62673d Branch: refs/heads/master Commit: be62673d9c0ae6778972f857cf55aef5b62a1065 Parents: 7ac7762 Author: Edward Fancher <[email protected]> Authored: Fri Jun 9 13:38:53 2017 -0700 Committer: Mike Percy <[email protected]> Committed: Tue Jun 13 00:58:51 2017 +0000 ---------------------------------------------------------------------- src/kudu/security/tls_socket.cc | 3 +++ src/kudu/util/net/socket.h | 7 +++++++ 2 files changed, 10 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/be62673d/src/kudu/security/tls_socket.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_socket.cc b/src/kudu/security/tls_socket.cc index dbe5c68..7aeca31 100644 --- a/src/kudu/security/tls_socket.cc +++ b/src/kudu/security/tls_socket.cc @@ -77,6 +77,9 @@ Status TlsSocket::Writev(const struct ::iovec *iov, int iov_len, int32_t *nwritt int32_t frame_size = iov[i].iov_len; // Don't return before unsetting TCP_CORK. write_status = Write(static_cast<uint8_t*>(iov[i].iov_base), frame_size, nwritten); + if (!write_status.ok()) break; + + // nwritten should have the correct amount written. total_written += *nwritten; if (*nwritten < frame_size) break; } http://git-wip-us.apache.org/repos/asf/kudu/blob/be62673d/src/kudu/util/net/socket.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/net/socket.h b/src/kudu/util/net/socket.h index ce5b7bb..7c04ff6 100644 --- a/src/kudu/util/net/socket.h +++ b/src/kudu/util/net/socket.h @@ -120,8 +120,15 @@ class Socket { // get the error status using getsockopt(2) Status GetSockError() const; + // Write up to 'amt' bytes from 'buf' to the socket. The number of bytes + // actually written will be stored in 'nwritten'. If an error is returned, + // the value of 'nwritten' is undefined. virtual Status Write(const uint8_t *buf, int32_t amt, int32_t *nwritten); + // Vectorized Write. + // If there is an error, that error needs to be resolved before calling again. + // If there was no error, but not all the bytes were written, the unwritten + // bytes must be retried. See writev(2) for more information. virtual Status Writev(const struct ::iovec *iov, int iov_len, int32_t *nwritten); // Blocking Write call, returns IOError unless full buffer is sent.
