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

mojo/mojo.gyp

@@ -246,6 +246,7 @@
         'system/data_pipe_unittest.cc',
         'system/dispatcher_unittest.cc',
         'system/local_data_pipe_unittest.cc',
+        'system/memory_unittest.cc',
         'system/message_pipe_dispatcher_unittest.cc',
         'system/message_pipe_unittest.cc',
         'system/multiprocess_message_pipe_unittest.cc',

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;

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;

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) ||

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(

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

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
 

mojo/system/memory_unittest.cc

+// 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

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;
   }
 

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))