Closed Bug 1450231 Opened 2 years ago Closed 2 years ago
_CRASH in Fatal Error() for --enable-fuzzing builds
No description provided.
I think we should also disable the |ProtocolErrorBreakpoint| at the top of |FatalError|.
Comment on attachment 8964489 [details] [diff] [review] fatalerror.diff Review of attachment 8964489 [details] [diff] [review]: ----------------------------------------------------------------- Technically this isn't a problem, since it won't affect non-FUZZING builds. I was concerned that it might cause problems, at least in the form of findings that don't make sense and don't apply to real builds, because it might get into state space that normally can't happen, and in particular because code that uses IPC tends to depend on errors being immediately fatal on the child side and doesn't handle errors. So I looked through IPC error handling, and if I'm following all the method calls correctly that's not a problem — this patch applies only to incoming messages that fail deserialization, and quietly dropping those should be fine, because an attacker could just not send them. But I'm a little confused, because I'd expect the MsgValueError to end up in the toplevel actor's ProcessingError method, which also kills the child process, and I'd expect that to cause the same problems for fuzzing as these crashes. (We do, I think, want to keep that behavior for the MsgProcessingError case when a handler returns false, but that's handled through IPCResult::Fail.) In any case, r=me, and sorry for taking so long with the review. ::: ipc/glue/ProtocolUtils.cpp @@ +280,1 @@ > ProtocolErrorBreakpoint(aMsg); This shouldn't cause problems, and it could be helpful: it just logs a message that there was an error. (It's named “breakpoint”, I assume, because it's marked noinline so it can be set as a debugger breakpoint.)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7a5b7274d8 Ignore MOZ_CRASH in FatalError() for --enable-fuzzing builds. r=jld
You need to log in before you can comment on or make changes to this bug.