Skip to content
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

[release/8.0-staging] Disable W^X in Rosetta emulated x64 containers on macOS #105117

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jul 18, 2024

Backport of #102509 to release/8.0-staging

/cc @janvorli

Customer Impact

The docker on macOS Arm64 uses Rosetta to run x64 containers. That has an effect on the double mapping. The Rosetta is unable to detect when an already executed code page is modified. So we cannot use double mapping on those containers. To detect that case, this change adds check that verifies that the double mapping works even when the code is modified.

Regression

  • Yes
  • No

Testing

The fix was verified locally on macOS using docker. The issue was not seen before because we don't test Linux x64 containers on arm64 macOS.

Risk

Low, this fix was in main for two months without causing any problems.

The docker on macOS Arm64 uses Rosetta to run x64 containers. That
has an effect on the double mapping. The Rosetta is unable to detect
when an already executed code page is modified. So we cannot use double
mapping on those containers. To detect that case, this change adds check
that verifies that the double mapping works even when the code is
modified.

Close #102226
This will help WINE running 32 bit code under rosetta emulation on
macOS.
@janvorli janvorli requested a review from jkotas July 18, 2024 21:57
@janvorli janvorli self-assigned this Jul 18, 2024
@janvorli janvorli added this to the 8.0.x milestone Jul 18, 2024
@janvorli janvorli added the Servicing-consider Issue for next servicing release review label Jul 18, 2024
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 8.0.x

@carlossanlop
Copy link
Member

@janvorli @jeffschwMSFT I'm about to close the servicing branches. Should we get this merged to include it in the September release or do we want to wait until October?

@jeffschwMSFT
Copy link
Member

unless sent in email and approved tonight, it will be for the next release

@carlossanlop
Copy link
Member

unless sent in email and approved tonight, it will be for the next release

Oh, I was asking because it had the servicing-approved label, so I assumed it already went through Tactics. But I tried to look for the approval email and I could not find one. I'll revert it to servicing-consider, @jeffschwMSFT.

@janvorli can you please send an email to Tactics requesting approval?

@carlossanlop carlossanlop added Servicing-consider Issue for next servicing release review and removed Servicing-approved Approved for servicing release labels Aug 13, 2024
@jeffschwMSFT
Copy link
Member

apologies I got the issues mixed up, this was approved in tactics last week. switching labels back

@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 13, 2024
@carlossanlop
Copy link
Member

carlossanlop commented Aug 13, 2024

apologies I got the issues mixed up, this was approved in tactics last week. switching labels back

@jeffschwMSFT so do we want to get it included in the September Release? I can just cherry-pick this change. Asking because I already flowed everything from staging to internal: #106301

@jeffschwMSFT
Copy link
Member

if possible, yes. it was my mistake on not getting it merged earlier

@carlossanlop carlossanlop merged commit 53780c9 into release/8.0-staging Aug 13, 2024
186 of 195 checks passed
@carlossanlop
Copy link
Member

/backport to release/8.0

@carlossanlop carlossanlop deleted the backport/pr-102509-to-release/8.0-staging branch August 13, 2024 17:38
Copy link
Contributor Author

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/10374534934

@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
@rbhanda rbhanda modified the milestones: 8.0.9, 8.0.10 Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants