IonBuilder::computeHeapType can return stack memory

RESOLVED FIXED in Firefox 56

Status

()

P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: gps, Assigned: sstangl)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
According to clang's static analysis, https://hg.mozilla.org/mozilla-central/file/da986c9f1f72/js/src/jit/IonBuilder.cpp#l9741 appears to return stack-allocated memory.

nbp says this is a false positive because "basically TypeSet::unionSets allocates a new TemporaryTypeSet, and the guards above the acc are guarding that we enter the loop."

Since this is identified as a severe vulnerability by static analysis, it would be great if the code could be rewritten to avoid the false positive.
Flags: needinfo?(nicolas.b.pierron)
Might indeed be good to get this fixed. I think it should be straight forward to fix? Let's do it. Putting this as P2 to get it this or next release.
Priority: -- → P2
Priority: P2 → P3
(Assignee)

Comment 2

a year ago
Created attachment 8888933 [details] [diff] [review]
0001-Reorganize-code-that-looks-like-it-could-return-a-st.patch

Browsing through old bugs.
Attachment #8888933 - Flags: review?(nicolas.b.pierron)
Attachment #8888933 - Flags: review?(nicolas.b.pierron) → review+
Flags: needinfo?(nicolas.b.pierron)
(Assignee)

Updated

a year ago
Keywords: checkin-needed
Assignee: nobody → sstangl

Comment 3

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e1e06adf80f
Reorganize code that looks like it could return a stack address. r=nbp
Keywords: checkin-needed

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8e1e06adf80f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.