[Security Review][Fuzzing][LangFuzz] Replace YARR with irregexp

RESOLVED FIXED

Status

mozilla.org
Security Assurance: Review Request
--
major
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

Details

(Whiteboard: [Fx])

(Assignee)

Description

3 years ago
JS fuzzing was requested for bug 976446. Goal is to find and fix potential regressions before landing.
(Assignee)

Comment 1

3 years ago
Testing this on langfuzz1 now with 32/64 bit debug+opt and opt builds.

Brian, I assume this will also need ARM specific testing right? Should I run this patch also through the ARM simulator?
Flags: needinfo?(bhackett1024)
Eventually, yeah, but right now ARM (including the simulator) seems to be broken; I'll fix this by early next week.
Flags: needinfo?(bhackett1024)
(Assignee)

Comment 3

3 years ago
Found four issues in the first round of fuzzing, second round requested. Second round should also include ASan fuzzing to find potential problems within irregexp itself.

Brian, is the ARM issue fixed now so we can test this on ARM as well?
Flags: needinfo?(bhackett1024)
(In reply to Christian Holler (:decoder) from comment #3)
> Found four issues in the first round of fuzzing, second round requested.
> Second round should also include ASan fuzzing to find potential problems
> within irregexp itself.
> 
> Brian, is the ARM issue fixed now so we can test this on ARM as well?

I've run this in the ARM simulator and I get a couple MOZ_CRASH()'s on jit-tests in Simulator-arm.cpp code (at lines 1939 and 3999 fwiw).  I don't know if this is due to bugs in the patch or to limitations in the simulator.  Jan, do you know what could be the problem here?  Anyways, I think this is fine to go ahead and test on ARM.
Flags: needinfo?(bhackett1024) → needinfo?(jdemooij)
(In reply to Brian Hackett (:bhackett) from comment #4)
> I've run this in the ARM simulator and I get a couple MOZ_CRASH()'s on
> jit-tests in Simulator-arm.cpp code (at lines 1939 and 3999 fwiw).  I don't
> know if this is due to bugs in the patch or to limitations in the simulator.
> Jan, do you know what could be the problem here?  Anyways, I think this is
> fine to go ahead and test on ARM.

Did your ARM fixes (bug 976446 comment 31) address this too?
Flags: needinfo?(jdemooij) → needinfo?(bhackett1024)
(In reply to Jan de Mooij [:jandem] from comment #5)
> (In reply to Brian Hackett (:bhackett) from comment #4)
> Did your ARM fixes (bug 976446 comment 31) address this too?

Yes, this was due to the ABI bug when growing the backtrack stack that caused us to end up executing random memory and make the simulator angry.
Flags: needinfo?(bhackett1024)
(Assignee)

Comment 7

3 years ago
This landed :)
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
This hasn't landed yet, so far just a small build break fixing patch has gone in.
You need to log in before you can comment on or make changes to this bug.