Closed Bug 1803334 Opened 1 year ago Closed 7 months ago

TestDllInterceptor is broken under active Intel CET

Categories

(Core :: mozglue, defect)

x86_64
Windows
defect

Tracking

()

RESOLVED FIXED
119 Branch
Tracking Status
firefox119 --- fixed

People

(Reporter: yannis, Assigned: yannis)

References

Details

Attachments

(1 file)

Although the tested code itself is compatible with Intel CET, the current code for TestDllInterceptor isn't. It ends up calling MovPushRet in 64-bit builds, which is not compatible with Intel CET: the shadow stack ensures that we ret to a return address that was pushed by a call. The test thus crashes on my local machine. I guess we have the same problem with PushRet in 32-bit builds.

I believe we should keep CET active (if available) in the test in order to maximize our chances to catch CET failures in the tested code. Indeed, the tested code is a typical place where we might be introducing CET failures without noticing; I almost introduced CET failures in it myself recently (see bug 1798787 comment 13). This would only have been caught if letting CET active in the test and testing on a CET-enabled machine.

In TestDllInterceptor, we should thus try to detect whether CET is active, and avoid executing the problematic instructions in that case. This bug also raises the question whether we should have test workers with CET active?

Assignee: nobody → yjuglaret

The patch needs a more recent SDK (10.0.19041.0, which is the version we officially support, works); workers are building with 10.0.17134.0 at the moment.

Depends on: 1774628

The severity field is not set for this bug.
:glandium, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mh+mozilla)
Severity: -- → S4
Flags: needinfo?(mh+mozilla)
Depends on: 1832467
No longer depends on: 1774628
Pushed by yjuglaret@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/00ccd5b7965e
Skip execution of MovPushRet and PushRet in TestDllInterceptor if Intel CET is active. r=handyman
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 119 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: