From 9b9132c715225faa4be288a18d72169bda40513c Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Mon, 13 Dec 2021 23:36:38 +0000 Subject: [PATCH] Fix OOB access while dumping struct layout for invalid storage class layout A one letter typo would lead to invalid memory access in the very specific case of outputting the layout for a struct within a struct with field alignment padding, and the inner struct has more members than the outer. Bug: tint:1344 Bug: oss-fuzz:72642 Change-Id: I749e3fb172e78a20ece68b40be1a0a57dc5746f4 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72642 Reviewed-by: Ben Clayton Reviewed-by: David Neto Kokoro: Kokoro Commit-Queue: Antonio Maiorano --- src/resolver/resolver_validation.cc | 5 +- .../storage_class_layout_validation_test.cc | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/resolver/resolver_validation.cc b/src/resolver/resolver_validation.cc index 6921a51543..117eb25688 100644 --- a/src/resolver/resolver_validation.cc +++ b/src/resolver/resolver_validation.cc @@ -263,7 +263,8 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str, // TODO(amaiorano): Output struct and member decorations so that this output // can be copied verbatim back into source - auto get_struct_layout_string = [&](const sem::Struct* st) -> std::string { + auto get_struct_layout_string = [this, member_name_of, type_name_of]( + const sem::Struct* st) -> std::string { std::stringstream ss; if (st->Members().empty()) { @@ -308,7 +309,7 @@ bool Resolver::ValidateStorageClassLayout(const sem::Struct* str, auto* const m = st->Members()[i]; // Output field alignment padding, if any - auto* const prev_member = (i == 0) ? nullptr : str->Members()[i - 1]; + auto* const prev_member = (i == 0) ? nullptr : st->Members()[i - 1]; if (prev_member) { uint32_t padding = m->Offset() - (prev_member->Offset() + prev_member->Size()); diff --git a/src/resolver/storage_class_layout_validation_test.cc b/src/resolver/storage_class_layout_validation_test.cc index 3ad7d3f1fc..467608d2f5 100644 --- a/src/resolver/storage_class_layout_validation_test.cc +++ b/src/resolver/storage_class_layout_validation_test.cc @@ -263,6 +263,63 @@ TEST_F(ResolverStorageClassLayoutValidationTest, 22:24 note: see declaration of variable)"); } +// See https://crbug.com/tint/1344 +TEST_F(ResolverStorageClassLayoutValidationTest, + UniformBuffer_MembersOffsetNotMultipleOf16_InnerMoreMembersThanOuter) { + // struct Inner { + // a : i32; + // b : i32; + // c : i32; + // [[align(1), size(5)]] scalar : i32; + // }; + // + // [[block]] + // struct Outer { + // inner : Inner; + // scalar : i32; + // }; + // + // [[group(0), binding(0)]] + // var a : Outer; + + Structure(Source{{12, 34}}, "Inner", + { + Member("a", ty.i32()), + Member("b", ty.i32()), + Member("c", ty.i32()), + Member("scalar", ty.i32(), {MemberAlign(1), MemberSize(5)}), + }); + + Structure(Source{{34, 56}}, "Outer", + { + Member(Source{{56, 78}}, "inner", ty.type_name("Inner")), + Member(Source{{78, 90}}, "scalar", ty.i32()), + }, + {StructBlock()}); + + Global(Source{{22, 24}}, "a", ty.type_name("Outer"), + ast::StorageClass::kUniform, GroupAndBinding(0, 0)); + + ASSERT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + R"(78:90 error: uniform storage requires that the number of bytes between the start of the previous member of type struct and the current member be a multiple of 16 bytes, but there are currently 20 bytes between 'inner' and 'scalar'. Consider setting [[align(16)]] on this member +34:56 note: see layout of struct: +/* align(4) size(24) */ struct Outer { +/* offset( 0) align(4) size(20) */ inner : Inner; +/* offset(20) align(4) size( 4) */ scalar : i32; +/* */ }; +12:34 note: and layout of previous member struct: +/* align(4) size(20) */ struct Inner { +/* offset( 0) align(4) size( 4) */ a : i32; +/* offset( 4) align(4) size( 4) */ b : i32; +/* offset( 8) align(4) size( 4) */ c : i32; +/* offset(12) align(1) size( 5) */ scalar : i32; +/* offset(17) align(1) size( 3) */ // -- implicit struct size padding --; +/* */ }; +22:24 note: see declaration of variable)"); +} + TEST_F(ResolverStorageClassLayoutValidationTest, UniformBuffer_MembersOffsetNotMultipleOf16_SuggestedFix) { // struct Inner {