Skip to content

[release/5.0-rc2] Make EventPipeProviderCallbackData own the filter data - #42368

Merged
davmason merged 3 commits into
release/5.0-rc2from
backport/pr-42307-to-release/5.0-rc2
Sep 18, 2020
Merged

[release/5.0-rc2] Make EventPipeProviderCallbackData own the filter data#42368
davmason merged 3 commits into
release/5.0-rc2from
backport/pr-42307-to-release/5.0-rc2

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Sep 17, 2020

Copy link
Copy Markdown
Contributor

Backport of #42307 to release/5.0-rc2

/cc @davmason

Customer Impact

There is a use after free bug in the EventPipe disable path, specifically in the code that deals with filter data. If a session specifies filter data for a provider they have a chance that the runtime could crash when disabling the session.

Testing

To test this change I modified the runtime to purposefully overwrite the filter data string immediately before it was freed in the destructor of EventPipeSessionProvider. Before this change you can see the modified string, after my change the filter data is unchanged.

Risk

Low - the fix is small, easy to reason about, and well understood.

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved. Please ask @noahfalk to do a cr.

@jeffschwMSFTjeffschwMSFT added the Servicing-consider Issue for next servicing release review label Sep 17, 2020
@jeffschwMSFTjeffschwMSFT added this to the 5.0.0 milestone Sep 17, 2020
@jeffschwMSFTjeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 17, 2020
@jeffschwMSFT

Copy link
Copy Markdown
Member

Approved for RC2 via email

@noahfalknoahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@davmason
davmason merged commit 202cef0 into release/5.0-rc2Sep 18, 2020
@jkotas
jkotas deleted the backport/pr-42307-to-release/5.0-rc2 branch September 19, 2020 16:59
@ghostghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Tracing-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@jeffschwMSFT@noahfalk@hoyosjs@davmason@Dotnet-GitSync-Bot