Checking for benign exceptions that trigger a minidump.

Checking for benign exceptions that trigger a minidump.

If the exception reponsible for the crash is benign, such as a floating point
exception, we can rule out the possibility that the code is exploitable. This
CL checks for such exceptions and marks the dump as not exploitable if such an
exception is found.

R=ivanpe@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1467

[liuandrew, 2015-06-26 16:14:45]

liu.andrew.x@gmail.com changed reviewers:
+ ahonig@google.com, ivanpe@chromium.org, srutherford@google.com

[ivanpe, 2015-06-26 18:21:36]

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.cc (right):

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:100: const MDRawExceptionStream
*raw_exception_stream = exception->exception();
Please, move this code new below the InstructionPointerInCode check.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:125: // check for benign exceptions
Capitalize, punctuation, etc.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:130: // TODO(liuandrew) change default
exploitability rating
Should be // TODO(author): some text

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:159: // here we check the cause of crash
Capitalize, punctuation, etc.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:163: case MD_EXCEPTION_CODE_LIN_SIGHUP:
What is the rationale for considering these benign?  If there a document that
describe these?  Can you add a link to it in a comment?

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.h (right):

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.h:56: bool BenignCrashTrigger(const
MDRawExceptionStream *raw_exception_stream);
Please, add a function description.

[liuandrew, 2015-06-29 16:13:56]

Fixed issues involving capitalization and punctuation. Unresolved issues include
the exception white-list and choosing a default exploitability level.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.cc (right):

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:100: const MDRawExceptionStream
*raw_exception_stream = exception->exception();
On 2015/06/26 18:21:36, ivanpe wrote:
> Please, move this code new below the InstructionPointerInCode check.

Done.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:125: // check for benign exceptions
On 2015/06/26 18:21:36, ivanpe wrote:
> Capitalize, punctuation, etc.

Done.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:130: // TODO(liuandrew) change default
exploitability rating
On 2015/06/26 18:21:36, ivanpe wrote:
> Should be // TODO(author): some text

Done.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.cc:159: // here we check the cause of crash
On 2015/06/26 18:21:36, ivanpe wrote:
> Capitalize, punctuation, etc.

Done.

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.h (right):

https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.h:56: bool BenignCrashTrigger(const
MDRawExceptionStream *raw_exception_stream);
On 2015/06/26 18:21:36, ivanpe wrote:
> Please, add a function description.

Done.

[ivanpe, 2015-06-29 18:06:19]

On 2015/06/29 16:13:56, liuandrew wrote:
> Fixed issues involving capitalization and punctuation. Unresolved issues
include
> the exception white-list and choosing a default exploitability level.
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> File src/processor/exploitability_linux.cc (right):
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> src/processor/exploitability_linux.cc:100: const MDRawExceptionStream
> *raw_exception_stream = exception->exception();
> On 2015/06/26 18:21:36, ivanpe wrote:
> > Please, move this code new below the InstructionPointerInCode check.
> 
> Done.
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> src/processor/exploitability_linux.cc:125: // check for benign exceptions
> On 2015/06/26 18:21:36, ivanpe wrote:
> > Capitalize, punctuation, etc.
> 
> Done.
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> src/processor/exploitability_linux.cc:130: // TODO(liuandrew) change default
> exploitability rating
> On 2015/06/26 18:21:36, ivanpe wrote:
> > Should be // TODO(author): some text
> 
> Done.
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> src/processor/exploitability_linux.cc:159: // here we check the cause of crash
> On 2015/06/26 18:21:36, ivanpe wrote:
> > Capitalize, punctuation, etc.
> 
> Done.
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> File src/processor/exploitability_linux.h (right):
> 
>
https://codereview.chromium.org/1212383004/diff/1/src/processor/exploitabilit...
> src/processor/exploitability_linux.h:56: bool BenignCrashTrigger(const
> MDRawExceptionStream *raw_exception_stream);
> On 2015/06/26 18:21:36, ivanpe wrote:
> > Please, add a function description.
> 
> Done.

LGTM

[liuandrew, 2015-06-30 16:31:15]

Hello Ivan, I added a more comprehensive list of whitelisted exceptions and
changed the default rating the EXPLOITABILITY_INTERESTING. All changes are
mirrored in Critique.

This should (hopefully) be the last set of changes to this CL. Can I get an LGTM
from you in both Chromium and Critique before I commit this CL? Thanks.

[ivanpe, 2015-06-30 17:35:06]

LGTM

[liuandrew, 2015-06-30 20:34:54]

Committed patchset #5 (id:80001) manually as r1467 (presubmit successful).