Skip to content

Conversation

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Oct 25, 2024

This API is inherently unsafe because it doesn't provide a length. All uses are replaced with MethodDesc::GetSigPointer or MethodDesc::GetSigParser. Both of which ensure bounds are respected during any parsing.

This API is inherently unsafe because it doesn't provide a length.
All uses are replaced with MethodDesc::GetSigPointer or MethodDesc::GetSigParser.
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 10.0.0 milestone Oct 25, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Remove MethodDesc::GetSig. Remove MethodDesc::GetSig Oct 25, 2024
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.


BYTE callConv = sig.GetCallingConventionInfo();
if (callConv != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT))
COMPlusThrow(kInvalidProgramException);
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, does this use HMF or is it already on the asm helper plan under the hood?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't use HMF since this must be called when an actual transition is performed - a QCall. The QCall erects a normal transition frame to unmanaged. Note that SuppressGCTransition on a QCall makes this call invalid. The COMPlusThrow APIs hook into the normal .NET exception mechanism here. I'm not the expert in this domain so if you need more details @jkotas or @janvorli are best to comment on how it aligns with the referenced approach/comment.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 661fd5e into dotnet:main Oct 25, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the remove_md_getsig branch October 25, 2024 15:45
@github-actions github-actions bot locked and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants