Skip to content

Add tests for ref field overlap with OBJECTREF and non-aligned ref fields. #64643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/coreclr/vm/methodtablebuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8391,7 +8391,6 @@ MethodTableBuilder::HandleExplicitLayout(
{
STANDARD_VM_CONTRACT;


// Instance slice size is the total size of an instance, and is calculated as
// the field whose offset and size add to the greatest number.
UINT instanceSliceSize = 0;
Expand Down Expand Up @@ -8426,13 +8425,13 @@ MethodTableBuilder::HandleExplicitLayout(
pFieldLayout[i] = empty;
}

// go through each field and look for invalid layout
// Go through each field and look for invalid layout.
// (note that we are more permissive than what Ecma allows. We only disallow the minimum set necessary to
// close security holes.)
//
// This is what we implment:
// This is what we implement:
//
// 1. Verify that every OREF is on a valid alignment
// 1. Verify that every OREF or BYREF is on a valid alignment.
// 2. Verify that OREFs only overlap with other OREFs.
// 3. If an OREF does overlap with another OREF, the class is marked unverifiable.
// 4. If an overlap of any kind occurs, the class will be marked NotTightlyPacked (affects ValueType.Equals()).
Expand All @@ -8445,7 +8444,6 @@ MethodTableBuilder::HandleExplicitLayout(
isObject[i] = oref;
}


ExplicitClassTrust explicitClassTrust;

UINT valueClassCacheIndex = ((UINT)(-1));
Expand Down Expand Up @@ -8481,17 +8479,22 @@ MethodTableBuilder::HandleExplicitLayout(
// "i" indexes all fields, valueClassCacheIndex indexes non-static fields only. Don't get them confused!
valueClassCacheIndex++;

if (CorTypeInfo::IsObjRef(pFD->GetFieldType()))
CorElementType type = pFD->GetFieldType();
if (CorTypeInfo::IsObjRef(type) || CorTypeInfo::IsByRef(type))
{
// Check that the ref offset is pointer aligned
if ((pFD->GetOffset_NoLogging() & ((ULONG)TARGET_POINTER_SIZE - 1)) != 0)
{
badOffset = pFD->GetOffset_NoLogging();
fieldTrust.SetTrust(ExplicitFieldTrust::kNone);

// If we got here, OREF field was not pointer aligned. THROW.
// If we got here, OREF or BYREF field was not pointer aligned. THROW.
break;
}
}

if (CorTypeInfo::IsObjRef(type))
{
// check if overlaps another object
if (memcmp((void *)&pFieldLayout[pFD->GetOffset_NoLogging()], (void *)isObject, sizeof(isObject)) == 0)
{
Expand Down
4 changes: 4 additions & 0 deletions src/mono/mono/metadata/class-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@ mono_class_setup_fields (MonoClass *klass)
mono_class_set_type_load_failure (klass, "Missing field layout info for %s", field->name);
break;
}
if (m_type_is_byref (field->type) && (offset % MONO_ABI_ALIGNOF (gpointer) != 0)) {
mono_class_set_type_load_failure (klass, "Field '%s' has an invalid offset", field->name);
break;
}
if (offset < -1) { /*-1 is used to encode special static fields */
mono_class_set_type_load_failure (klass, "Field '%s' has a negative offset %d", field->name, offset);
break;
Expand Down
34 changes: 27 additions & 7 deletions src/tests/Loader/classloader/RefFields/InvalidCSharp.il
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,46 @@

.assembly InvalidCSharp { }

.class public auto ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithRefField
.class public sequential ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithRefField
extends [System.Runtime]System.ValueType
{
// Type requires IsByRefLikeAttribute to be valid.
.field public string& invalid
.field public string& Invalid
}

.class public explicit ansi sealed beforefieldinit InvalidCSharp.InvalidRefFieldAlignment
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
01 00 00 00
)
.field [0] public int16 Field
.field [2] public int32& Invalid
}

.class public explicit ansi sealed beforefieldinit InvalidCSharp.InvalidObjectRefRefFieldOverlap
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
01 00 00 00
)
.field [0] public object Field
.field [0] public int32& Invalid
}

// This is invalid metadata and is unable to be loaded.
// - [field sig] (0x80131815 (VER_E_FIELD_SIG))
//
// .class public auto ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithStaticRefField
// .class public sequential ansi sealed beforefieldinit InvalidCSharp.InvalidStructWithStaticRefField
// extends [System.Runtime]System.ValueType
// {
// .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
// 01 00 00 00
// )
// .field public static string& invalid
// .field public static string& Invalid
// }

.class public auto ansi sealed beforefieldinit InvalidCSharp.WithRefField
.class public sequential ansi sealed beforefieldinit InvalidCSharp.WithRefField
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
Expand Down Expand Up @@ -57,7 +77,7 @@
}
}

.class public auto ansi sealed beforefieldinit InvalidCSharp.WithRefStructField
.class public sequential ansi sealed beforefieldinit InvalidCSharp.WithRefStructField
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
Expand Down Expand Up @@ -91,7 +111,7 @@
}
}

.class public auto ansi sealed beforefieldinit InvalidCSharp.WithTypedReferenceField`1<T>
.class public sequential ansi sealed beforefieldinit InvalidCSharp.WithTypedReferenceField`1<T>
extends [System.Runtime]System.ValueType
{
.custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = (
Expand Down
2 changes: 2 additions & 0 deletions src/tests/Loader/classloader/RefFields/Validate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public static void Validate_Invalid_RefField_Fails()
{
Console.WriteLine($"{nameof(Validate_Invalid_RefField_Fails)}...");
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidStructWithRefField); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidRefFieldAlignment); });
Assert.Throws<TypeLoadException>(() => { var t = typeof(InvalidObjectRefRefFieldOverlap); });
}

[Fact]
Expand Down