Commit 5f90f5a3 authored by yzshen@chromium.org's avatar yzshen@chromium.org

Add validation logic for non-nullable types.

This CL only turns on the non-nullable check for validation tests.

There will be separate CLs for:
- add DCHECK at the sending side.
- make the existing APIs pass the non-nullable check and turn on the check everywhere.

BUG=324170
TEST=New and revised validation tests.

Review URL: https://codereview.chromium.org/466613002

Cr-Commit-Position: refs/heads/master@{#290002}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290002 0039d316-1c4b-4281-b951-d872f2087c98
parent 5f8501db
......@@ -52,19 +52,5 @@ void ArraySerializationHelper<Handle, true>::DecodePointersAndHandles(
DecodeHandle(&elements[i], handles);
}
// static
bool ArraySerializationHelper<Handle, true>::ValidateElements(
const ArrayHeader* header,
const ElementType* elements,
BoundsChecker* bounds_checker) {
for (uint32_t i = 0; i < header->num_elements; ++i) {
if (!bounds_checker->ClaimHandle(elements[i])) {
ReportValidationError(VALIDATION_ERROR_ILLEGAL_HANDLE);
return false;
}
}
return true;
}
} // namespace internal
} // namespace mojo
......@@ -8,10 +8,12 @@
#include <new>
#include <vector>
#include "mojo/public/c/system/macros.h"
#include "mojo/public/cpp/bindings/lib/bindings_internal.h"
#include "mojo/public/cpp/bindings/lib/bindings_serialization.h"
#include "mojo/public/cpp/bindings/lib/bounds_checker.h"
#include "mojo/public/cpp/bindings/lib/buffer.h"
#include "mojo/public/cpp/bindings/lib/template_util.h"
#include "mojo/public/cpp/bindings/lib/validation_errors.h"
#include "mojo/public/cpp/environment/logging.h"
......@@ -110,11 +112,32 @@ struct ArrayDataTraits<bool> {
}
};
// Array type information needed for valdiation.
template <uint32_t in_expected_num_elements,
bool in_element_nullable,
typename InElementValidateParams>
class ArrayValidateParams {
public:
// Validation information for elements. It is either another specialization of
// ArrayValidateParams (if elements are arrays) or NoValidateParams.
typedef InElementValidateParams ElementValidateParams;
// If |expected_num_elements| is not 0, the array is expected to have exactly
// that number of elements.
static const uint32_t expected_num_elements = in_expected_num_elements;
// Whether the elements are nullable.
static const bool element_nullable = in_element_nullable;
};
// NoValidateParams is used to indicate the end of an ArrayValidateParams chain.
class NoValidateParams {
};
// What follows is code to support the serialization of Array_Data<T>. There
// are two interesting cases: arrays of primitives and arrays of objects.
// Arrays of objects are represented as arrays of pointers to objects.
template <typename T, bool kIsHandle> struct ArraySerializationHelper;
template <typename T, bool is_handle> struct ArraySerializationHelper;
template <typename T>
struct ArraySerializationHelper<T, false> {
......@@ -130,9 +153,15 @@ struct ArraySerializationHelper<T, false> {
std::vector<Handle>* handles) {
}
template <bool element_nullable, typename ElementValidateParams>
static bool ValidateElements(const ArrayHeader* header,
const ElementType* elements,
BoundsChecker* bounds_checker) {
MOJO_COMPILE_ASSERT(!element_nullable,
Primitive_type_should_be_non_nullable);
MOJO_COMPILE_ASSERT(
(IsSame<ElementValidateParams, NoValidateParams>::value),
Primitive_type_should_not_have_array_validate_params);
return true;
}
};
......@@ -149,9 +178,27 @@ struct ArraySerializationHelper<Handle, true> {
ElementType* elements,
std::vector<Handle>* handles);
template <bool element_nullable, typename ElementValidateParams>
static bool ValidateElements(const ArrayHeader* header,
const ElementType* elements,
BoundsChecker* bounds_checker);
BoundsChecker* bounds_checker) {
MOJO_COMPILE_ASSERT(
(IsSame<ElementValidateParams, NoValidateParams>::value),
Handle_type_should_not_have_array_validate_params);
for (uint32_t i = 0; i < header->num_elements; ++i) {
if (IsNonNullableValidationEnabled() && !element_nullable &&
elements[i].value() == kEncodedInvalidHandleValue) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE);
return false;
}
if (!bounds_checker->ClaimHandle(elements[i])) {
ReportValidationError(VALIDATION_ERROR_ILLEGAL_HANDLE);
return false;
}
}
return true;
}
};
template <typename H>
......@@ -172,11 +219,13 @@ struct ArraySerializationHelper<H, true> {
header, elements, handles);
}
template <bool element_nullable, typename ElementValidateParams>
static bool ValidateElements(const ArrayHeader* header,
const ElementType* elements,
BoundsChecker* bounds_checker) {
return ArraySerializationHelper<Handle, true>::ValidateElements(
header, elements, bounds_checker);
return ArraySerializationHelper<Handle, true>::
ValidateElements<element_nullable, ElementValidateParams>(
header, elements, bounds_checker);
}
};
......@@ -198,19 +247,46 @@ struct ArraySerializationHelper<P*, false> {
Decode(&elements[i], handles);
}
template <bool element_nullable, typename ElementValidateParams>
static bool ValidateElements(const ArrayHeader* header,
const ElementType* elements,
BoundsChecker* bounds_checker) {
for (uint32_t i = 0; i < header->num_elements; ++i) {
if (IsNonNullableValidationEnabled() && !element_nullable &&
!elements[i].offset) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
return false;
}
if (!ValidateEncodedPointer(&elements[i].offset)) {
ReportValidationError(VALIDATION_ERROR_ILLEGAL_POINTER);
return false;
}
if (!P::Validate(DecodePointerRaw(&elements[i].offset), bounds_checker))
if (!ValidateCaller<P, ElementValidateParams>::Run(
DecodePointerRaw(&elements[i].offset), bounds_checker)) {
return false;
}
}
return true;
}
private:
template <typename T, typename Params>
struct ValidateCaller {
static bool Run(const void* data, BoundsChecker* bounds_checker) {
MOJO_COMPILE_ASSERT(
(IsSame<Params, NoValidateParams>::value),
Struct_type_should_not_have_array_validate_params);
return T::Validate(data, bounds_checker);
}
};
template <typename T, typename Params>
struct ValidateCaller<Array_Data<T>, Params> {
static bool Run(const void* data, BoundsChecker* bounds_checker) {
return Array_Data<T>::template Validate<Params>(data, bounds_checker);
}
};
};
template <typename T>
......@@ -229,11 +305,8 @@ class Array_Data {
num_elements);
}
// If expected_num_elements is not zero, the actual number of elements in the
// header must match that value or the message is rejected.
static bool Validate(const void* data,
BoundsChecker* bounds_checker,
uint32_t expected_num_elements = 0) {
template <typename Params>
static bool Validate(const void* data, BoundsChecker* bounds_checker) {
if (!data)
return true;
if (!IsAligned(data)) {
......@@ -250,8 +323,8 @@ class Array_Data {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER);
return false;
}
if (expected_num_elements != 0 &&
header->num_elements != expected_num_elements) {
if (Params::expected_num_elements != 0 &&
header->num_elements != Params::expected_num_elements) {
ReportValidationError(VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER);
return false;
}
......@@ -261,8 +334,9 @@ class Array_Data {
}
const Array_Data<T>* object = static_cast<const Array_Data<T>*>(data);
return Helper::ValidateElements(&object->header_, object->storage(),
bounds_checker);
return Helper::template ValidateElements<
Params::element_nullable, typename Params::ElementValidateParams>(
&object->header_, object->storage(), bounds_checker);
}
size_t size() const { return header_.num_elements; }
......
......@@ -28,8 +28,12 @@ const char* ValidationErrorToString(ValidationError error) {
return "VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER";
case VALIDATION_ERROR_ILLEGAL_HANDLE:
return "VALIDATION_ERROR_ILLEGAL_HANDLE";
case VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE:
return "VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE";
case VALIDATION_ERROR_ILLEGAL_POINTER:
return "VALIDATION_ERROR_ILLEGAL_POINTER";
case VALIDATION_ERROR_UNEXPECTED_NULL_POINTER:
return "VALIDATION_ERROR_UNEXPECTED_NULL_POINTER";
case VALIDATION_ERROR_MESSAGE_HEADER_INVALID_FLAG_COMBINATION:
return "VALIDATION_ERROR_MESSAGE_HEADER_INVALID_FLAG_COMBINATION";
case VALIDATION_ERROR_MESSAGE_HEADER_MISSING_REQUEST_ID:
......@@ -50,6 +54,8 @@ ValidationErrorObserverForTesting::ValidationErrorObserverForTesting()
: last_error_(VALIDATION_ERROR_NONE) {
MOJO_DCHECK(!g_validation_error_observer);
g_validation_error_observer = this;
MOJO_LOG(WARNING) << "Non-nullable validation is turned on for testing but "
<< "not for production code yet!";
}
ValidationErrorObserverForTesting::~ValidationErrorObserverForTesting() {
......@@ -57,5 +63,9 @@ ValidationErrorObserverForTesting::~ValidationErrorObserverForTesting() {
g_validation_error_observer = NULL;
}
bool IsNonNullableValidationEnabled() {
return !!g_validation_error_observer;
}
} // namespace internal
} // namespace mojo
......@@ -28,11 +28,17 @@ enum ValidationError {
// An array header doesn't make sense, for example:
// - |num_bytes| is smaller than the size of the header plus the size required
// to store |num_elements| elements.
// - For fixed-size arrays, |num_elements| is different than the specified
// size.
VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER,
// An encoded handle is illegal.
VALIDATION_ERROR_ILLEGAL_HANDLE,
// A non-nullable handle field is set to invalid handle.
VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE,
// An encoded pointer is illegal.
VALIDATION_ERROR_ILLEGAL_POINTER,
// A non-nullable pointer field is set to null.
VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
// |flags| in the message header is an invalid flag combination.
VALIDATION_ERROR_MESSAGE_HEADER_INVALID_FLAG_COMBINATION,
// |flags| in the message header indicates that a request ID is required but
......@@ -60,6 +66,14 @@ class ValidationErrorObserverForTesting {
MOJO_DISALLOW_COPY_AND_ASSIGN(ValidationErrorObserverForTesting);
};
// Currently it only returns true when there is a
// ValidationErrorObserverForTesting object alive. In other words, non-nullable
// validation is only turned on during validation tests.
//
// TODO(yzshen): Remove this function and enable non-nullable validation by
// default.
bool IsNonNullableValidationEnabled();
} // namespace internal
} // namespace mojo
......
......@@ -6,5 +6,5 @@
[dist4]method1_params // num_bytes
[u4]1 // num_fields
[u8]0 // param0: A null pointer.
[u8]0 // param0: An unexpected null pointer.
[anchr]method1_params
......@@ -6,5 +6,5 @@
[dist4]method3_params // num_bytes
[u4]1 // num_fields
[u8]0 // param0: A null pointer.
[u8]0 // param0: An unexpected null pointer.
[anchr]method3_params
......@@ -8,8 +8,29 @@
[dist4]method5_params // num_bytes
[u4]2 // num_fields
[u8]0 // param0
[dist8]param0_ptr // param0
[u4]10 // param1: It is outside of the valid encoded handle
// range [0, 9).
// range [0, 10).
[u4]0 // padding
[anchr]method5_params
[anchr]param0_ptr
[dist4]struct_e // num_bytes
[u4]2 // num_fields
[dist8]struct_d_ptr // struct_d
[u4]3 // data_pipe_consumer
[u4]0 // padding
[anchr]struct_e
[anchr]struct_d_ptr
[dist4]struct_d // num_bytes
[u4]1 // num_fields
[dist8]message_pipes_ptr // message_pipes
[anchr]struct_d
[anchr]message_pipes_ptr
[dist4]message_pipe_array // num_bytes
[u4]2 // num_elements
[u4]0
[u4]1
[anchr]message_pipe_array
......@@ -16,7 +16,20 @@
[anchr]param0_ptr
[dist4]struct_e // num_bytes
[u4]2 // num_fields
[u8]0 // struct_d
[dist8]struct_d_ptr // struct_d
[u4]4 // data_pipe_consumer: The same value as |param1| above.
[u4]0 // padding
[anchr]struct_e
[anchr]struct_d_ptr
[dist4]struct_d // num_bytes
[u4]1 // num_fields
[dist8]message_pipes_ptr // message_pipes
[anchr]struct_d
[anchr]message_pipes_ptr
[dist4]message_pipe_array // num_bytes
[u4]2 // num_elements
[u4]0
[u4]1
[anchr]message_pipe_array
[handles]0
[handles]5
[dist4]message_header // num_bytes
[u4]2 // num_fields
......@@ -9,7 +9,7 @@
[dist4]method5_params // num_bytes
[u4]2 // num_fields
[dist8]param0_ptr // param0
[s4]-1 // param1: An invalid handle.
[u4]4 // param1
[u4]0 // padding
[anchr]method5_params
......@@ -17,7 +17,7 @@
[dist4]struct_e // num_bytes
[u4]2 // num_fields
[dist8]struct_d_ptr // struct_d
[s4]-1 // data_pipe_consumer: An invalid handle.
[s4]-1 // data_pipe_consumer: An unexpected invalid handle.
[u4]0 // padding
[anchr]struct_e
......@@ -29,8 +29,6 @@
[anchr]message_pipes_ptr
[dist4]message_pipe_array // num_bytes
[u4]3 // num_elements
[s4]-1 // An invalid handle.
[s4]-1 // An invalid handle.
[s4]-1 // An invalid handle.
[u4]1 // num_elements
[u4]2
[anchr]message_pipe_array
......@@ -8,14 +8,14 @@
[u4]2 // num_fields
[dist8]param0_ptr // param0
[dist8]param1_ptr // param1
[u8]0 // padding
[u8]0 // unknown
[anchr]method7_params
[anchr]param0_ptr
[dist4]struct_c // num_bytes
[dist4]struct_f // num_bytes
[u4]1 // num_fields
[dist8]array_ptr // array
[anchr]struct_c
[dist8]array_ptr // fixed_size_array
[anchr]struct_f
[anchr]array_ptr
[dist4]array_member // num_bytes
......
......@@ -8,22 +8,21 @@
[u4]2 // num_fields
[dist8]param0_ptr // param0
[dist8]param1_ptr // param1
[u8]0 // padding
[anchr]method7_params
[anchr]param0_ptr
[dist4]struct_c // num_bytes
[dist4]struct_f // num_bytes
[u4]1 // num_fields
[dist8]array_ptr // array
[anchr]struct_c
[dist8]array_ptr // fixed_size_array
[anchr]struct_f
[anchr]array_ptr
[dist4]array_member // num_bytes
[u4]2 // num_elements
[u4]2 // num_elements: Too few elements.
0 1
[anchr]array_member
[u4]0 [u1]0 [u1]0 // padding for alignment of next array
[u4]0 [u1]0 [u1]0 // Padding for alignment of next array.
[anchr]param1_ptr
[dist4]array_param // num_bytes
......
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]7 // name
[u4]0 // flags
[anchr]message_header
[dist4]method7_params // num_bytes
[u4]2 // num_fields
[dist8]param0_ptr // param0
[dist8]param1_ptr // param1
[anchr]method7_params
[anchr]param0_ptr
[dist4]struct_f // num_bytes
[u4]1 // num_fields
[u8]0 // fixed_size_array: An unexpected null pointer.
[anchr]struct_f
[anchr]param1_ptr
[dist4]array_param // num_bytes
[u4]5 // num_elements
0 1 2 3 4
[anchr]array_param
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]8 // name
[u4]0 // flags
[anchr]message_header
[dist4]method8_params // num_bytes
[u4]1 // num_fields
[dist8]param0_ptr // param0
[anchr]method8_params
[anchr]param0_ptr
[dist4]array_param // num_bytes
[u4]3 // num_elements
[u8]0 // A null pointer, which is okay.
[dist8]nested_array_ptr
[u8]0 // A null pointer, which is okay.
[anchr]array_param
[anchr]nested_array_ptr
[dist4]nested_array // num_bytes
[u4]1 // num_elements
[dist8]string_ptr
[anchr]nested_array
[anchr]string_ptr
[dist4]string // num_bytes
[u4]5 // num_elements
0 1 2 3 4
[anchr]string
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]8 // name
[u4]0 // flags
[anchr]message_header
[dist4]method8_params // num_bytes
[u4]1 // num_fields
[u8]0 // param0: An unexpected null pointer.
[anchr]method8_params
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]8 // name
[u4]0 // flags
[anchr]message_header
[dist4]method8_params // num_bytes
[u4]1 // num_fields
[dist8]param0_ptr // param0
[anchr]method8_params
[anchr]param0_ptr
[dist4]array_param // num_bytes
[u4]3 // num_elements
[u8]0 // A null pointer, which is okay.
[dist8]nested_array_ptr
[u8]0 // A null pointer, which is okay.
[anchr]array_param
[anchr]nested_array_ptr
[dist4]nested_array // num_bytes
[u4]2 // num_elements
[dist8]string_ptr
[u8]0 // An unexpected null pointer.
[anchr]nested_array
[anchr]string_ptr
[dist4]string // num_bytes
[u4]5 // num_elements
0 1 2 3 4
[anchr]string
[handles]4
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]9 // name
[u4]0 // flags
[anchr]message_header
[dist4]method9_params // num_bytes
[u4]1 // num_fields
[dist8]param0_ptr // param0
[anchr]method9_params
[anchr]param0_ptr
[dist4]array_param // num_bytes
[u4]2 // num_elements
[dist8]nested_array_ptr_0
[dist8]nested_array_ptr_1
[anchr]array_param
[anchr]nested_array_ptr_0
[dist4]nested_array_0 // num_bytes
[u4]2 // num_elements
[u4]0
[s4]-1 // An invalid handle, which is okay.
[anchr]nested_array_0
[anchr]nested_array_ptr_1
[dist4]nested_array_1 // num_bytes
[u4]3 // num_elements
[u4]2
[s4]-1 // An invalid handle, which is okay.
[u4]3
[anchr]nested_array_1
[handles]4
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]9 // name
[u4]0 // flags
[anchr]message_header
[dist4]method9_params // num_bytes
[u4]1 // num_fields
[u8]0 // param0: A null pointer, which is okay.
[anchr]method9_params
[handles]4
[dist4]message_header // num_bytes
[u4]2 // num_fields
[u4]9 // name
[u4]0 // flags
[anchr]message_header
[dist4]method9_params // num_bytes
[u4]1 // num_fields
[dist8]param0_ptr // param0
[anchr]method9_params
[anchr]param0_ptr
[dist4]array_param // num_bytes
[u4]2 // num_elements
[dist8]nested_array_ptr_0
[u8]0 // An unexpected null pointer.
[anchr]array_param
[anchr]nested_array_ptr_0
[dist4]nested_array_0 // num_bytes
[u4]1 // num_elements
[u4]0
[anchr]nested_array_0
......@@ -40,6 +40,8 @@ interface ConformanceTestInterface {
Method5(StructE param0, handle<data_pipe_producer> param1);
Method6(uint8[][] param0);
Method7(StructF param0, uint8[5] param1);
Method8(string[]?[] param0);
Method9(handle?[][]? param0);
};
struct BasicStruct {
......
......@@ -13,22 +13,42 @@
{%- for packed_field in struct.packed.packed_fields %}
{%- set name = packed_field.field.name %}
{%- if packed_field.field.kind|is_object_kind %}
{%- set wrapper_type = packed_field.field.kind|cpp_wrapper_type %}
{%- set kind = packed_field.field.kind %}
{%- if kind|is_object_kind %}
{%- set wrapper_type = kind|cpp_wrapper_type %}
{%- if not kind|is_nullable_kind %}
if (mojo::internal::IsNonNullableValidationEnabled() &&
!object->{{name}}.offset) {
ReportValidationError(
mojo::internal::VALIDATION_ERROR_UNEXPECTED_NULL_POINTER);
return false;
}
{%- endif %}
if (!mojo::internal::ValidateEncodedPointer(&object->{{name}}.offset)) {
ReportValidationError(mojo::internal::VALIDATION_ERROR_ILLEGAL_POINTER);
return false;
}
{%- if kind|is_any_array_kind or kind|is_string_kind %}
if (!{{wrapper_type}}::Data_::Validate<
{{kind|get_array_validate_params|indent(10)}}>(
mojo::internal::DecodePointerRaw(&object->{{name}}.offset),
bounds_checker)) {
{%- else %}
if (!{{wrapper_type}}::Data_::Validate(
mojo::internal::DecodePointerRaw(&object->{{name}}.offset),
bounds_checker
{%- if packed_field.field.kind|is_any_array_kind -%}
, {{packed_field.field.kind|expected_array_size}}
{%- endif -%}
)) {
bounds_checker)) {
{%- endif %}
return false;
}
{%- elif packed_field.field.kind|is_any_handle_kind %}
{%- elif kind|is_any_handle_kind %}
{%- if not kind|is_nullable_kind %}
if (mojo::internal::IsNonNullableValidationEnabled() &&
object->{{name}}.value() == mojo::internal::kEncodedInvalidHandleValue) {
ReportValidationError(
mojo::internal::VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE);
return false;
}
{%- endif %}
if (!bounds_checker->ClaimHandle(object->{{name}})) {
ReportValidationError(mojo::internal::VALIDATION_ERROR_ILLEGAL_HANDLE);
return false;
......
......@@ -243,6 +243,24 @@ def ShouldInlineStruct(struct):
return False
return True
def GetArrayValidateParams(kind):
if not mojom.IsAnyArrayKind(kind) and not mojom.IsStringKind(kind):
return "mojo::internal::NoValidateParams"
if mojom.IsStringKind(kind):
expected_num_elements = 0
element_nullable = False
element_validate_params = "mojo::internal::NoValidateParams"
else:
expected_num_elements = generator.ExpectedArraySize(kind)
element_nullable = mojom.IsNullableKind(kind.kind)
element_validate_params = GetArrayValidateParams(kind.kind)
return "mojo::internal::ArrayValidateParams<%d, %s,\n%s> " % (
expected_num_elements,
'true' if element_nullable else 'false',
element_validate_params)
_HEADER_SIZE = 8
class Generator(generator.Generator):
......@@ -258,6 +276,7 @@ class Generator(generator.Generator):
"default_value": DefaultValue,
"expected_array_size": generator.ExpectedArraySize,
"expression_to_text": ExpressionToText,
"get_array_validate_params": GetArrayValidateParams,
"get_name_for_kind": GetNameForKind,
"get_pad": pack.GetPad,
"has_callbacks": HasCallbacks,
......@@ -268,6 +287,7 @@ class Generator(generator.Generator):
"is_any_handle_kind": mojom.IsAnyHandleKind,
"is_interface_kind": mojom.IsInterfaceKind,
"is_interface_request_kind": mojom.IsInterfaceRequestKind,
"is_nullable_kind": mojom.IsNullableKind,
"is_object_kind": mojom.IsObjectKind,
"is_string_kind": mojom.IsStringKind,
"is_struct_with_handles": IsStructWithHandles,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment