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

Status: RESOLVED DUPLICATE of bug 642327

(Core :: JavaScript Engine, enhancement)

Description

[Paul Biggar]

Attachment: Add OOM checking to |make check|

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).

Comment 1

[Paul Biggar]

Review ping? I just want to know if there is a security problem with adding OOM checks to |make check|, not a full review.

Comment 2

[David Mandelin [:dmandelin]]

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?

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 4

[Jeff Walden [:Waldo]]

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 5

[David Mandelin [:dmandelin]]

Comment on attachment 513625 [details] [diff] [review]
Add OOM checking to |make check|

Looks like we have broad agreement.

Comment 6

[Paul Biggar]

Great.