diff options
author | Harvey Tuch <htuch@google.com> | 2017-01-20 11:02:11 -0500 |
---|---|---|
committer | Harvey Tuch <htuch@google.com> | 2017-02-07 15:45:44 -0500 |
commit | 5f3cfe960f708a397c4465a3064072dd3b2d7bb7 (patch) | |
tree | 363b85675839116c4314dbff8548170d94031acf /test | |
parent | 27ee9d015dff36ebf32e95c80a4d0b37ac7137ba (diff) |
Fix read from uninitialized memory bug in GrpcBufferWriter.
This commit fixes an issue in which the following sequence of operations
leads to use of uninitialized memory:
1. Caller invokes GrpcBufferWriter::Next(), and then makes use of 8191
bytes in the returned buffer (which is 8192 bytes in size).
2. Caller then returns the unused single byte via
GrpcBufferWriter::BackUp(). This method invokes
g_core_codegen_interface->grpc_slice_split_tail(), which causes
backup_slice_ to be a grpc_slice with one byte.
3. At the next invocation of GrpcBufferWriter::Next(), a reference to
the single byte grpc_slice is returned to the caller.
The problem here is that the returned reference is to the inlined buffer
in the grpc_slice, which is resident in slice_, not the location of the
buffer inside slice_buffer_ after
g_core_codegen_interface->grpc_slice_buffer_add() in
GrpcBufferWriter::Next(). As a result, any data the caller writes to the
returned void* data is lost.
The solution is to avoid inlined backup slices.
Diffstat (limited to 'test')
-rw-r--r-- | test/cpp/codegen/codegen_test_full.cc | 2 | ||||
-rw-r--r-- | test/cpp/codegen/proto_utils_test.cc | 93 |
2 files changed, 94 insertions, 1 deletions
diff --git a/test/cpp/codegen/codegen_test_full.cc b/test/cpp/codegen/codegen_test_full.cc index d6e2416b55..bc19fc9669 100644 --- a/test/cpp/codegen/codegen_test_full.cc +++ b/test/cpp/codegen/codegen_test_full.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2016, Google Inc. + * Copyright 2017, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/cpp/codegen/proto_utils_test.cc b/test/cpp/codegen/proto_utils_test.cc new file mode 100644 index 0000000000..1daa142b50 --- /dev/null +++ b/test/cpp/codegen/proto_utils_test.cc @@ -0,0 +1,93 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include <grpc++/impl/codegen/proto_utils.h> +#include <grpc++/impl/grpc_library.h> +#include <gtest/gtest.h> + +namespace grpc { +namespace internal { + +static GrpcLibraryInitializer g_gli_initializer; + +// Provide access to GrpcBufferWriter internals. +class GrpcBufferWriterPeer { + public: + explicit GrpcBufferWriterPeer(internal::GrpcBufferWriter* writer) + : writer_(writer) {} + bool have_backup() const { return writer_->have_backup_; } + const grpc_slice& backup_slice() const { return writer_->backup_slice_; } + const grpc_slice& slice() const { return writer_->slice_; } + + private: + GrpcBufferWriter* writer_; +}; + +class ProtoUtilsTest : public ::testing::Test {}; + +// Regression test for a memory corruption bug where a series of +// GrpcBufferWriter Next()/Backup() invocations could result in a dangling +// pointer returned by Next() due to the interaction between grpc_slice inlining +// and GRPC_SLICE_START_PTR. +TEST_F(ProtoUtilsTest, BackupNext) { + // Ensure the GrpcBufferWriter internals are initialized. + g_gli_initializer.summon(); + + grpc_byte_buffer* bp; + GrpcBufferWriter writer(&bp, 8192); + GrpcBufferWriterPeer peer(&writer); + + void* data; + int size; + // Allocate a slice. + ASSERT_TRUE(writer.Next(&data, &size)); + EXPECT_EQ(8192, size); + // Return a single byte. Before the fix that this test acts as a regression + // for, this would have resulted in an inlined backup slice. + writer.BackUp(1); + EXPECT_TRUE(!peer.have_backup()); + // On the next allocation, the slice is non-inlined. + ASSERT_TRUE(writer.Next(&data, &size)); + EXPECT_TRUE(peer.slice().refcount != NULL); + + // Cleanup. + g_core_codegen_interface->grpc_byte_buffer_destroy(bp); +} + +} // namespace internal +} // namespace grpc + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} |