Closed Bug 1630607 Opened 11 months ago Closed 9 months ago

arm64: add debug-only high-bits-are-zero check in boxValue

Categories

(Core :: JavaScript Engine: JIT, enhancement, P1)

ARM64
Unspecified
enhancement

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

(Keywords: sec-other, Whiteboard: [keep hidden while bug 1631508 is][post-critsmash-triage][adv-main78-])

Attachments

(1 file, 2 obsolete files)

This would have been quite useful to have to catch a bug in the new arm64 backend, and the x64 backend does it, so let's do it on arm64 too.

Priority: -- → P1
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/60f939d5d3bc
arm64: check that upper high bits are zero'd when boxing a 32-bit value; r=jandem

Backed out changeset 60f939d5d3bc (bug 1630607) for SM failure at workspace/breakpad-tools/libbreakpadinjector.so

Backout: https://hg.mozilla.org/integration/autoland/rev/aaa6e44260125781accde50de4db5aeefeb3011b

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=60f939d5d3bc56097df7ee60bd58703b75969e53

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298056758&repo=autoland&lineNumber=18849

[task 2020-04-17T07:54:36.676Z] TEST-PASS | non262/String/split-01.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.2 s]
[task 2020-04-17T07:54:36.682Z] ## non262/String/ropes.js: rc = -2, run time = 0.26164
[task 2020-04-17T07:54:36.683Z] Stress test of ropes
[task 2020-04-17T07:54:36.683Z] TEST-UNEXPECTED-FAIL | non262/String/ropes.js | (args: "--dll /builds/worker/workspace/breakpad-tools/libbreakpadinjector.so") [0.3 s]

Flags: needinfo?(bbouvier)
Group: core-security, javascript-core-security

Hiding until we understand what's going on. The failure of this check could mean that a JSValue's type is badly reinterpreted, which can be one step to arbitrary code execution.

Group: core-security

Does anyone in the JIT team want to investigate the failure here (maybe showing a sec issue)? I'm running out of bandwidth here, and it might take a week or two before I can get back to this.

Flags: needinfo?(jdemooij)

This is very likely a real sec issue. We emit a 64-bit LEA instruction on ARM64 when we should be using a 32-bit one. I'm doing a bit more digging to see if there are any mitigating factors that limit the severity, but so far it looks bad :(

I did some more digging and it looks like we can construct arbitrary type-confusion here. There is an ARM64-only issue.

Hardware: Unspecified → ARM64

I'm moving the fix to a new bug so it is easier to track. https://bugzilla.mozilla.org/show_bug.cgi?id=1631508

Comment on attachment 9141714 [details]
Bug 1630607 - Fix LEffectiveAddress on ARM64

Revision D71567 was moved to bug 1631508. Setting attachment 9141714 [details] to obsolete.

Attachment #9141714 - Attachment is obsolete: true
Flags: needinfo?(jdemooij)

Thanks Ted for looking at it!

Flags: needinfo?(bbouvier)
Keywords: sec-other
See Also: → 1631508
Whiteboard: [keep hidden while bug 1631508 is]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bbouvier, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(bbouvier)

After discussing it with Jan, still waiting a bit on people updating so as to get the fix from bug 1631508 before landing this.

Flags: needinfo?(bbouvier)
Severity: -- → N/A

I think we can land this now :)

Flags: needinfo?(bbouvier)

Sounds good, will do a full, silent try-build first.

Try build returned green, after a few retriggers: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b61dbdcb602f2503073d2cae34abc752139f6e2e

Will rebase and queue for landing now.

Flags: needinfo?(bbouvier)
Attachment #9151662 - Attachment is obsolete: true
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Flags: qe-verify-
Whiteboard: [keep hidden while bug 1631508 is] → [keep hidden while bug 1631508 is][post-critsmash-triage]
Whiteboard: [keep hidden while bug 1631508 is][post-critsmash-triage] → [keep hidden while bug 1631508 is][post-critsmash-triage][adv-main78-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.