diff options
author | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-03 10:37:16 +0000 |
---|---|---|
committer | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-03 10:37:16 +0000 |
commit | 4adc127a9ca34a1b4059fcab8b69e8b140c77579 (patch) | |
tree | 5cb8bd7ed49d5e875e952b02d4b68a43f5ddd5a7 /mojo/system | |
parent | 70cf6ccd9a4565a39b1bd98c4f0a99bdb48d5941 (diff) | |
download | chromium_src-4adc127a9ca34a1b4059fcab8b69e8b140c77579.zip chromium_src-4adc127a9ca34a1b4059fcab8b69e8b140c77579.tar.gz chromium_src-4adc127a9ca34a1b4059fcab8b69e8b140c77579.tar.bz2 |
Mojo: Specify/check alignment of pointers more carefully.
Improve the verification routines especially in the case that no count
is necessary.
Also add a test.
R=darin@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273936
Review URL: https://codereview.chromium.org/304303006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274461 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'mojo/system')
-rw-r--r-- | mojo/system/core.cc | 35 | ||||
-rw-r--r-- | mojo/system/core_test_base.cc | 4 | ||||
-rw-r--r-- | mojo/system/data_pipe_consumer_dispatcher.cc | 8 | ||||
-rw-r--r-- | mojo/system/data_pipe_producer_dispatcher.cc | 8 | ||||
-rw-r--r-- | mojo/system/memory.cc | 67 | ||||
-rw-r--r-- | mojo/system/memory.h | 55 | ||||
-rw-r--r-- | mojo/system/memory_unittest.cc | 136 | ||||
-rw-r--r-- | mojo/system/message_pipe_dispatcher.cc | 6 | ||||
-rw-r--r-- | mojo/system/shared_buffer_dispatcher.cc | 6 |
9 files changed, 263 insertions, 62 deletions
diff --git a/mojo/system/core.cc b/mojo/system/core.cc index 6c8cb83..f35284b 100644 --- a/mojo/system/core.cc +++ b/mojo/system/core.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/time/time.h" +#include "mojo/public/c/system/macros.h" #include "mojo/system/constants.h" #include "mojo/system/data_pipe.h" #include "mojo/system/data_pipe_consumer_dispatcher.h" @@ -126,9 +127,9 @@ MojoResult Core::WaitMany(const MojoHandle* handles, const MojoWaitFlags* flags, uint32_t num_handles, MojoDeadline deadline) { - if (!VerifyUserPointer<MojoHandle>(handles, num_handles)) + if (!VerifyUserPointerWithCount<MojoHandle>(handles, num_handles)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<MojoWaitFlags>(flags, num_handles)) + if (!VerifyUserPointerWithCount<MojoWaitFlags>(flags, num_handles)) return MOJO_RESULT_INVALID_ARGUMENT; if (num_handles < 1) return MOJO_RESULT_INVALID_ARGUMENT; @@ -139,9 +140,9 @@ MojoResult Core::WaitMany(const MojoHandle* handles, MojoResult Core::CreateMessagePipe(MojoHandle* message_pipe_handle0, MojoHandle* message_pipe_handle1) { - if (!VerifyUserPointer<MojoHandle>(message_pipe_handle0, 1)) + if (!VerifyUserPointer<MojoHandle>(message_pipe_handle0)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<MojoHandle>(message_pipe_handle1, 1)) + if (!VerifyUserPointer<MojoHandle>(message_pipe_handle1)) return MOJO_RESULT_INVALID_ARGUMENT; scoped_refptr<MessagePipeDispatcher> dispatcher0(new MessagePipeDispatcher()); @@ -198,7 +199,7 @@ MojoResult Core::WriteMessage(MojoHandle message_pipe_handle, // validity, even for dispatchers that don't support |WriteMessage()| and will // simply return failure unconditionally. It also breaks the usual // left-to-right verification order of arguments.) - if (!VerifyUserPointer<MojoHandle>(handles, num_handles)) + if (!VerifyUserPointerWithCount<MojoHandle>(handles, num_handles)) return MOJO_RESULT_INVALID_ARGUMENT; if (num_handles > kMaxMessageNumHandles) return MOJO_RESULT_RESOURCE_EXHAUSTED; @@ -251,9 +252,9 @@ MojoResult Core::ReadMessage(MojoHandle message_pipe_handle, return MOJO_RESULT_INVALID_ARGUMENT; if (num_handles) { - if (!VerifyUserPointer<uint32_t>(num_handles, 1)) + if (!VerifyUserPointer<uint32_t>(num_handles)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<MojoHandle>(handles, *num_handles)) + if (!VerifyUserPointerWithCount<MojoHandle>(handles, *num_handles)) return MOJO_RESULT_INVALID_ARGUMENT; } @@ -294,15 +295,16 @@ MojoResult Core::CreateDataPipe(const MojoCreateDataPipeOptions* options, MojoHandle* data_pipe_consumer_handle) { if (options) { // The |struct_size| field must be valid to read. - if (!VerifyUserPointer<uint32_t>(&options->struct_size, 1)) + if (!VerifyUserPointer<uint32_t>(&options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; // And then |options| must point to at least |options->struct_size| bytes. - if (!VerifyUserPointer<void>(options, options->struct_size)) + if (!VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>(options, + options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; } - if (!VerifyUserPointer<MojoHandle>(data_pipe_producer_handle, 1)) + if (!VerifyUserPointer<MojoHandle>(data_pipe_producer_handle)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<MojoHandle>(data_pipe_consumer_handle, 1)) + if (!VerifyUserPointer<MojoHandle>(data_pipe_consumer_handle)) return MOJO_RESULT_INVALID_ARGUMENT; MojoCreateDataPipeOptions validated_options = { 0 }; @@ -413,13 +415,14 @@ MojoResult Core::CreateSharedBuffer( MojoHandle* shared_buffer_handle) { if (options) { // The |struct_size| field must be valid to read. - if (!VerifyUserPointer<uint32_t>(&options->struct_size, 1)) + if (!VerifyUserPointer<uint32_t>(&options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; // And then |options| must point to at least |options->struct_size| bytes. - if (!VerifyUserPointer<void>(options, options->struct_size)) + if (!VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>(options, + options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; } - if (!VerifyUserPointer<MojoHandle>(shared_buffer_handle, 1)) + if (!VerifyUserPointer<MojoHandle>(shared_buffer_handle)) return MOJO_RESULT_INVALID_ARGUMENT; MojoCreateSharedBufferOptions validated_options = { 0 }; @@ -456,7 +459,7 @@ MojoResult Core::DuplicateBufferHandle( return MOJO_RESULT_INVALID_ARGUMENT; // Don't verify |options| here; that's the dispatcher's job. - if (!VerifyUserPointer<MojoHandle>(new_buffer_handle, 1)) + if (!VerifyUserPointer<MojoHandle>(new_buffer_handle)) return MOJO_RESULT_INVALID_ARGUMENT; scoped_refptr<Dispatcher> new_dispatcher; @@ -485,7 +488,7 @@ MojoResult Core::MapBuffer(MojoHandle buffer_handle, if (!dispatcher) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<void*>(buffer, 1)) + if (!VerifyUserPointerWithCount<void*>(buffer, 1)) return MOJO_RESULT_INVALID_ARGUMENT; scoped_ptr<RawSharedBufferMapping> mapping; diff --git a/mojo/system/core_test_base.cc b/mojo/system/core_test_base.cc index e4efe45..85f44ac 100644 --- a/mojo/system/core_test_base.cc +++ b/mojo/system/core_test_base.cc @@ -54,7 +54,7 @@ class MockDispatcher : public Dispatcher { info_->IncrementWriteMessageCallCount(); lock().AssertAcquired(); - if (!VerifyUserPointer<void>(bytes, num_bytes)) + if (!VerifyUserPointerWithSize<1>(bytes, num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; if (num_bytes > kMaxMessageNumBytes) return MOJO_RESULT_RESOURCE_EXHAUSTED; @@ -74,7 +74,7 @@ class MockDispatcher : public Dispatcher { info_->IncrementReadMessageCallCount(); lock().AssertAcquired(); - if (num_bytes && !VerifyUserPointer<void>(bytes, *num_bytes)) + if (num_bytes && !VerifyUserPointerWithSize<1>(bytes, *num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; return MOJO_RESULT_OK; diff --git a/mojo/system/data_pipe_consumer_dispatcher.cc b/mojo/system/data_pipe_consumer_dispatcher.cc index 409db29..2a2242c 100644 --- a/mojo/system/data_pipe_consumer_dispatcher.cc +++ b/mojo/system/data_pipe_consumer_dispatcher.cc @@ -56,7 +56,7 @@ MojoResult DataPipeConsumerDispatcher::ReadDataImplNoLock( MojoReadDataFlags flags) { lock().AssertAcquired(); - if (!VerifyUserPointer<uint32_t>(num_bytes, 1)) + if (!VerifyUserPointer<uint32_t>(num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; if ((flags & MOJO_READ_DATA_FLAG_DISCARD)) { @@ -75,7 +75,7 @@ MojoResult DataPipeConsumerDispatcher::ReadDataImplNoLock( } // Only verify |elements| if we're neither discarding nor querying. - if (!VerifyUserPointer<void>(elements, *num_bytes)) + if (!VerifyUserPointerWithSize<1>(elements, *num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; return data_pipe_->ConsumerReadData( @@ -88,9 +88,9 @@ MojoResult DataPipeConsumerDispatcher::BeginReadDataImplNoLock( MojoReadDataFlags flags) { lock().AssertAcquired(); - if (!VerifyUserPointer<const void*>(buffer, 1)) + if (!VerifyUserPointerWithCount<const void*>(buffer, 1)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<uint32_t>(buffer_num_bytes, 1)) + if (!VerifyUserPointer<uint32_t>(buffer_num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; // These flags may not be used in two-phase mode. if ((flags & MOJO_READ_DATA_FLAG_DISCARD) || diff --git a/mojo/system/data_pipe_producer_dispatcher.cc b/mojo/system/data_pipe_producer_dispatcher.cc index 574a0c2..617f231 100644 --- a/mojo/system/data_pipe_producer_dispatcher.cc +++ b/mojo/system/data_pipe_producer_dispatcher.cc @@ -56,9 +56,9 @@ MojoResult DataPipeProducerDispatcher::WriteDataImplNoLock( MojoWriteDataFlags flags) { lock().AssertAcquired(); - if (!VerifyUserPointer<uint32_t>(num_bytes, 1)) + if (!VerifyUserPointer<uint32_t>(num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<void>(elements, *num_bytes)) + if (!VerifyUserPointerWithSize<1>(elements, *num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; return data_pipe_->ProducerWriteData( @@ -71,9 +71,9 @@ MojoResult DataPipeProducerDispatcher::BeginWriteDataImplNoLock( MojoWriteDataFlags flags) { lock().AssertAcquired(); - if (!VerifyUserPointer<void*>(buffer, 1)) + if (!VerifyUserPointerWithCount<void*>(buffer, 1)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<uint32_t>(buffer_num_bytes, 1)) + if (!VerifyUserPointer<uint32_t>(buffer_num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; return data_pipe_->ProducerBeginWriteData( diff --git a/mojo/system/memory.cc b/mojo/system/memory.cc index dd8b3d2..498ca0b 100644 --- a/mojo/system/memory.cc +++ b/mojo/system/memory.cc @@ -7,28 +7,79 @@ #include <limits> #include "base/logging.h" +#include "build/build_config.h" namespace mojo { namespace system { -template <size_t size> -bool VerifyUserPointerForSize(const void* pointer, size_t count) { +namespace internal { + +template <size_t alignment> +bool IsAligned(const void* pointer) { + return reinterpret_cast<uintptr_t>(pointer) % alignment == 0; +} + +#if defined(COMPILER_MSVC) && defined(ARCH_CPU_32_BITS) +// MSVS (2010, 2013) sometimes (on the stack) aligns, e.g., |int64_t|s (for +// which |__alignof(int64_t)| is 8) to 4-byte boundaries. http://goo.gl/Y2n56T +template <> +bool IsAligned<8>(const void* pointer) { + return reinterpret_cast<uintptr_t>(pointer) % 4 == 0; +} +#endif + +template <size_t size, size_t alignment> +bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper(const void* pointer) { + // TODO(vtl): If running in kernel mode, do a full verification. For now, just + // check that it's non-null and aligned. (A faster user mode implementation is + // also possible if this check is skipped.) + return !!pointer && IsAligned<alignment>(pointer); +} + +// Explicitly instantiate the sizes we need. Add instantiations as needed. +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<1, 1>( + const void*); +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<4, 4>( + const void*); +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<8, 8>( + const void*); + +template <size_t size, size_t alignment> +bool VerifyUserPointerWithCountHelper(const void* pointer, size_t count) { if (count > std::numeric_limits<size_t>::max() / size) return false; // TODO(vtl): If running in kernel mode, do a full verification. For now, just - // check that it's non-null if |size| is nonzero. (A faster user mode - // implementation is also possible if this check is skipped.) - return count == 0 || !!pointer; + // check that it's non-null and aligned if |count| is nonzero. (A faster user + // mode implementation is also possible if this check is skipped.) + return count == 0 || (!!pointer && IsAligned<alignment>(pointer)); } // Explicitly instantiate the sizes we need. Add instantiations as needed. -template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerForSize<1>( +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<1, 1>( const void*, size_t); -template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerForSize<4>( +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<4, 4>( const void*, size_t); -template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerForSize<8>( +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<8, 8>( const void*, size_t); +} // nameespace internal + +template <size_t alignment> +bool VerifyUserPointerWithSize(const void* pointer, size_t size) { + // TODO(vtl): If running in kernel mode, do a full verification. For now, just + // check that it's non-null and aligned. (A faster user mode implementation is + // also possible if this check is skipped.) + return size == 0 || (!!pointer && internal::IsAligned<alignment>(pointer)); +} + +// Explicitly instantiate the alignments we need. Add instantiations as needed. +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<1>(const void*, + size_t); +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<4>(const void*, + size_t); +template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<8>(const void*, + size_t); + } // namespace system } // namespace mojo diff --git a/mojo/system/memory.h b/mojo/system/memory.h index 6483e03..21c36ea 100644 --- a/mojo/system/memory.h +++ b/mojo/system/memory.h @@ -7,39 +7,48 @@ #include <stddef.h> +#include "mojo/public/c/system/macros.h" #include "mojo/system/system_impl_export.h" namespace mojo { namespace system { -// This is just forward-declared, with the definition and explicit -// instantiations in the .cc file. This is used by |VerifyUserPointer<T>()| -// below, and you should use that instead. -template <size_t size> -bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerForSize(const void* pointer, - size_t count); - -// Verify that |count * sizeof(T)| bytes can be read from the user |pointer| -// insofar as possible/necessary (note: this is done carefully since |count * -// sizeof(T)| may overflow a |size_t|. |count| may be zero. If |T| is |void|, -// then the size of each element is taken to be a single byte. -// -// For example, if running in kernel mode, this should be a full verification -// that the given memory is owned and readable by the user process. In user -// mode, if crashes are acceptable, this may do nothing at all (and always -// return true). +namespace internal { + +template <size_t size, size_t alignment> +bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper(const void* pointer); + +template <size_t size, size_t alignment> +bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper( + const void* pointer, + size_t count); + +} // namespace internal + +// Verify (insofar as possible/necessary) that a |T| can be read from the user +// |pointer|. template <typename T> -bool VerifyUserPointer(const T* pointer, size_t count) { - return VerifyUserPointerForSize<sizeof(T)>(pointer, count); +bool VerifyUserPointer(const T* pointer) { + return internal::VerifyUserPointerHelper<sizeof(T), MOJO_ALIGNOF(T)>(pointer); } -// Special-case |T| equals |void| so that the size is in bytes, as indicated -// above. -template <> -inline bool VerifyUserPointer<void>(const void* pointer, size_t count) { - return VerifyUserPointerForSize<1>(pointer, count); +// Verify (insofar as possible/necessary) that |count| |T|s can be read from the +// user |pointer|; |count| may be zero. (This is done carefully since |count * +// sizeof(T)| may overflow a |size_t|.) +template <typename T> +bool VerifyUserPointerWithCount(const T* pointer, size_t count) { + return internal::VerifyUserPointerWithCountHelper<sizeof(T), + MOJO_ALIGNOF(T)>(pointer, + count); } +// Verify that |size| bytes (which may be zero) can be read from the user +// |pointer|, and that |pointer| has the specified |alignment| (if |size| is +// nonzero). +template <size_t alignment> +bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize(const void* pointer, + size_t size); + } // namespace system } // namespace mojo diff --git a/mojo/system/memory_unittest.cc b/mojo/system/memory_unittest.cc new file mode 100644 index 0000000..b23d50a --- /dev/null +++ b/mojo/system/memory_unittest.cc @@ -0,0 +1,136 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "mojo/system/memory.h" + +#include <stddef.h> +#include <stdint.h> + +#include <limits> + +#include "mojo/public/c/system/macros.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace mojo { +namespace system { +namespace { + +TEST(MemoryTest, Valid) { + char my_char; + int32_t my_int32; + int64_t my_int64; + char my_char_array[5]; + int32_t my_int32_array[5]; + int64_t my_int64_array[5]; + + // |VerifyUserPointer|: + + EXPECT_TRUE(VerifyUserPointer<char>(&my_char)); + EXPECT_TRUE(VerifyUserPointer<int32_t>(&my_int32)); + EXPECT_TRUE(VerifyUserPointer<int64_t>(&my_int64)); + + // |VerifyUserPointerWithCount|: + + EXPECT_TRUE(VerifyUserPointerWithCount<char>(my_char_array, 5)); + EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>(my_int32_array, 5)); + EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>(my_int64_array, 5)); + + // It shouldn't care what the pointer is if the count is zero. + EXPECT_TRUE(VerifyUserPointerWithCount<char>(NULL, 0)); + EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>(NULL, 0)); + EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>( + reinterpret_cast<const int32_t*>(1), 0)); + EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>(NULL, 0)); + EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>( + reinterpret_cast<const int64_t*>(1), 0)); + + // |VerifyUserPointerWithSize|: + + EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_char, sizeof(my_char))); + EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_int32, sizeof(my_int32))); + EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int32_t)>( + &my_int32, sizeof(my_int32))); + EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_int64, sizeof(my_int64))); + EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>( + &my_int64, sizeof(my_int64))); + + EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_char_array, + sizeof(my_char_array))); + EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_int32_array, + sizeof(my_int32_array))); + EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int32_t)>( + my_int32_array, sizeof(my_int32_array))); + EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_int64_array, + sizeof(my_int64_array))); + EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>( + my_int64_array, sizeof(my_int64_array))); +} + +TEST(MemoryTest, Invalid) { + // Note: |VerifyUserPointer...()| are defined to be "best effort" checks (and + // may always return true). Thus these tests of invalid cases only reflect the + // current implementation. + + // These tests depend on |int32_t| and |int64_t| having nontrivial alignment. + MOJO_COMPILE_ASSERT(MOJO_ALIGNOF(int32_t) != 1, + int32_t_does_not_have_to_be_aligned); + MOJO_COMPILE_ASSERT(MOJO_ALIGNOF(int64_t) != 1, + int64_t_does_not_have_to_be_aligned); + + int32_t my_int32; + int64_t my_int64; + + // |VerifyUserPointer|: + + EXPECT_FALSE(VerifyUserPointer<char>(NULL)); + EXPECT_FALSE(VerifyUserPointer<int32_t>(NULL)); + EXPECT_FALSE(VerifyUserPointer<int64_t>(NULL)); + + // Unaligned: + EXPECT_FALSE(VerifyUserPointer<int32_t>(reinterpret_cast<const int32_t*>(1))); + EXPECT_FALSE(VerifyUserPointer<int64_t>(reinterpret_cast<const int64_t*>(1))); + + // |VerifyUserPointerWithCount|: + + EXPECT_FALSE(VerifyUserPointerWithCount<char>(NULL, 1)); + EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>(NULL, 1)); + EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>(NULL, 1)); + + // Unaligned: + EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>( + reinterpret_cast<const int32_t*>(1), 1)); + EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>( + reinterpret_cast<const int64_t*>(1), 1)); + + // Count too big: + EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>( + &my_int32, std::numeric_limits<size_t>::max())); + EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>( + &my_int64, std::numeric_limits<size_t>::max())); + + // |VerifyUserPointerWithSize|: + + EXPECT_FALSE(VerifyUserPointerWithSize<1>(NULL, 1)); + EXPECT_FALSE(VerifyUserPointerWithSize<4>(NULL, 1)); + EXPECT_FALSE(VerifyUserPointerWithSize<4>(NULL, 4)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 1)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 4)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 8)); + + // Unaligned: + EXPECT_FALSE(VerifyUserPointerWithSize<4>(reinterpret_cast<const int32_t*>(1), + 1)); + EXPECT_FALSE(VerifyUserPointerWithSize<4>(reinterpret_cast<const int32_t*>(1), + 4)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1), + 1)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1), + 4)); + EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1), + 8)); +} + +} // namespace +} // namespace system +} // namespace mojo diff --git a/mojo/system/message_pipe_dispatcher.cc b/mojo/system/message_pipe_dispatcher.cc index 22bb3b3..89bc16a 100644 --- a/mojo/system/message_pipe_dispatcher.cc +++ b/mojo/system/message_pipe_dispatcher.cc @@ -150,7 +150,7 @@ MojoResult MessagePipeDispatcher::WriteMessageImplNoLock( lock().AssertAcquired(); - if (!VerifyUserPointer<void>(bytes, num_bytes)) + if (!VerifyUserPointerWithSize<1>(bytes, num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; if (num_bytes > kMaxMessageNumBytes) return MOJO_RESULT_RESOURCE_EXHAUSTED; @@ -168,9 +168,9 @@ MojoResult MessagePipeDispatcher::ReadMessageImplNoLock( lock().AssertAcquired(); if (num_bytes) { - if (!VerifyUserPointer<uint32_t>(num_bytes, 1)) + if (!VerifyUserPointer<uint32_t>(num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<void>(bytes, *num_bytes)) + if (!VerifyUserPointerWithSize<1>(bytes, *num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; } diff --git a/mojo/system/shared_buffer_dispatcher.cc b/mojo/system/shared_buffer_dispatcher.cc index 29f1de5..841c659 100644 --- a/mojo/system/shared_buffer_dispatcher.cc +++ b/mojo/system/shared_buffer_dispatcher.cc @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "mojo/public/c/system/macros.h" #include "mojo/system/constants.h" #include "mojo/system/memory.h" #include "mojo/system/raw_shared_buffer.h" @@ -149,10 +150,11 @@ MojoResult SharedBufferDispatcher::DuplicateBufferHandleImplNoLock( lock().AssertAcquired(); if (options) { // The |struct_size| field must be valid to read. - if (!VerifyUserPointer<uint32_t>(&options->struct_size, 1)) + if (!VerifyUserPointer<uint32_t>(&options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; // And then |options| must point to at least |options->struct_size| bytes. - if (!VerifyUserPointer<void>(options, options->struct_size)) + if (!VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>(options, + options->struct_size)) return MOJO_RESULT_INVALID_ARGUMENT; if (options->struct_size < sizeof(*options)) |