Closed Bug 635413 Opened 10 years ago Closed 10 years ago

Add OOM regression testing to |make check| (sg version)

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 642327

People

(Reporter: paul.biggar, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

This patch attempts to prevent regressions in our OOM handling. It is almost the same as bug 624094. I've made a separate bug to discuss whether this can be included in the tree from a security perspective.

Summary of the patch:
 - in DEBUG mode, add a |-A NUM| flag to the shell, causing memory allocation to fail on the NUM'th allocation
 - add a script to cycle through all memory allocations, and see if we fail properly
 - call the script from |make check|

The output of the script is:

  "Found the expected number of OOM errors (139)"


The problem is that this makes it easy to find OOM bugs. Is that a security problem? If so, is there some version of this bug that can be put into the repository to help prevent regressions.

(There is no need to look at non-security aspects of this just yet, I'll ask njn for a review before pushing).
Attachment #513625 - Flags: review?(dmandelin)
Summary: Add OOM regression checking to |make check| → Add OOM regression testing to |make check|
Whiteboard: [sg:nse]
Severity: normal → enhancement
Blocks: 636940
Blocks: 638208
Review ping? I just want to know if there is a security problem with adding OOM checks to |make check|, not a full review.
It seems OK to me: I figure anyone would could create an exploit from this would have no trouble creating this sort of test anyway. But we should get another opinion or two. David, Bill?
If we have any sort of plan to drive the OOM errors down to zero, then we should probably do that first and then take the patch. But it doesn't sound like there's much will to do that--some of these are pretty tricky to fix. In that case, I think it's better to take the patch now. There's a small chance somebody will find a way to exploit an existing bug that they wouldn't otherwise have known about. But I think that's outweighed by the benefit of additional test coverage.
I see no reason to hold this.  Other people have done this trick in the past, it's not hard to do, we shouldn't think not doing this provides any extra "security".
Comment on attachment 513625 [details] [diff] [review]
Add OOM checking to |make check|

Looks like we have broad agreement.
Attachment #513625 - Flags: review?(dmandelin) → review+
Great.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Summary: Add OOM regression testing to |make check| → Add OOM regression testing to |make check| (sg version)
Whiteboard: [sg:nse]
Duplicate of bug: 642327
Group: core-security
You need to log in before you can comment on or make changes to this bug.