Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Prevent loading byref-like types with invalid layout #25200

Merged
merged 1 commit into from
Jun 18, 2019

Conversation

MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented Jun 17, 2019

First approximation of a fix for #25057.

This has two problems:

  • We're checking for any byref-like typed fields. Types that don't actually contain interior pointers but were marked as ref struct will fail to load when not aligned properly.
  • We're not doing the deep validation that we do for reference types to make sure the ByReference<T> field doesn't overlap with another non-byreference field.

Question is whether we're okay with those limitations, or whether we need a better fix. Better fix would likely entail inefficiently walking over the fields à la FindByRefPointerOffsetsInByRefLikeObject (doing the more efficient thing that we do for object references below would require a GCDesc representation of byrefness).

Contributes to #25057.

@MichalStrehovsky
Copy link
Member Author

Cc @sergiy-k

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@MichalStrehovsky MichalStrehovsky merged commit 84dc373 into dotnet:master Jun 18, 2019
@MichalStrehovsky MichalStrehovsky deleted the invalidbyreflayout branch June 18, 2019 07:07
MichalStrehovsky added a commit to MichalStrehovsky/corert that referenced this pull request Jul 3, 2019
MichalStrehovsky added a commit to dotnet/corert that referenced this pull request Jul 3, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…25200)

First approximation of a fix for dotnet/coreclr#25057.

This has two problems:
* We're checking for any byref-like typed fields. Types that don't actually contain interior pointers but were marked as `ref struct` will fail to load when not aligned properly.
* We're not doing the deep validation that we do for reference types to make sure the `ByReference<T>` field doesn't overlap with another non-byreference field.

Question is whether we're okay with those limitations, or whether we need a better fix. Better fix would likely entail inefficiently walking over the fields à la `FindByRefPointerOffsetsInByRefLikeObject` (doing the more efficient thing that we do for object references below would require a GCDesc representation of byrefness).

Contributes to dotnet/coreclr#25057.

Commit migrated from dotnet/coreclr@84dc373
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants