[2/3] impala git commit: KUDU-2004. Undefined behavior in TlsSocket::Writev()

[ta...@apache.org]

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 <mp...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/9359
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/dd3b3db3
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/dd3b3db3
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/dd3b3db3

Branch: refs/heads/2.x
Commit: dd3b3db38b480bc60ac4c4764a3fea7f93a6f176
Parents: dab55a7
Author: Edward Fancher <ef...@cloudera.com>
Authored: Fri Jun 9 13:38:53 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Feb 20 09:10:13 2018 +0000

----------------------------------------------------------------------
 be/src/kudu/security/tls_socket.cc | 3 +++
 be/src/kudu/util/net/socket.h      | 7 +++++++
 2 files changed, 10 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/dd3b3db3/be/src/kudu/security/tls_socket.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/security/tls_socket.cc b/be/src/kudu/security/tls_socket.cc
index dbe5c68..7aeca31 100644
--- a/be/src/kudu/security/tls_socket.cc
+++ b/be/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/impala/blob/dd3b3db3/be/src/kudu/util/net/socket.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/util/net/socket.h b/be/src/kudu/util/net/socket.h
index ce5b7bb..7c04ff6 100644
--- a/be/src/kudu/util/net/socket.h
+++ b/be/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.