Closed Bug 744816 Opened 8 years ago Closed 8 years ago
Fennec crashes in concurrent access of password db
Seen with latest inbound build with password Sync http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android/1334228548/ STR's: 0) clean install the given build 1) Setup sync to existing desktop client via jpake 2) attempt to add a password to fennec at say bugzilla.mozilla.org tested results: fennec crashes and sync fails expected results: no crash, password is added to db and sync succeeds.
blocking-fennec1.0: --- → ?
The causes for those (by our current understanding) require it to be simultaneous with Gecko launching. That is not the case here?
Looks like an invalid free in GeckoJNI code. I blame wesj.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #3) > Looks like an invalid free in GeckoJNI code. I blame wesj. Quick sleuthing! Poor wesj.
Weird. I rewrote encrypt and decrypt to both do nothing but char* result = (char*)malloc(2048); free(result); return jenv->NewStringUTF(""); And I still crash.
(In reply to Wesley Johnston (:wesj) from comment #5) > And I still crash. As far as I know, this implies that the problem is more general heap corruption that is simply being detected during free. Time to audit all of the C++ that runs up until that point…
I have temporarily disabled the password sync stage until this bug is resolved. https://hg.mozilla.org/integration/mozilla-inbound/rev/bbfe59782089
Status: NEW → ASSIGNED
Priority: -- → P1
I don't like this much, but wanted to have this up as a bandaid patch while I try to figure this out. Basically, just holds a buffer forever. I can trigger this bug without having to have two threads with concurrent access to the db. It seems related to the length of the string I try to encode (although that also seems unlikely and probably is unrelated).
I knew this was something stupid staring me in the face.
Comment on attachment 615004 [details] [diff] [review] Fix Review of attachment 615004 [details] [diff] [review]: ----------------------------------------------------------------- Heh. Good spot. Can you write a trivial test that fails when linked with eFence or equivalent? Or file a bug to run cppcheck against this source? This kind of overrun should be easy to catch automatically, and who knows… there might be more.  <http://en.wikipedia.org/wiki/Electric_Fence>  <http://sourceforge.net/apps/mediawiki/cppcheck/index.php?title=Main_Page>
Attachment #615004 - Flags: review?(rnewman) → review+
Whiteboard: [native-crash] → [native-crash][has reviewed patch]
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef563bc49ec2 I'm looking into using efence (i don't see it used anywhere else in our source, but it seems like it would be a useful thing...). There's also some work using cppcheck (bug 679417), but no bug to automagically run it. I did run it here though. Filing bugs for things it shows...
https://gist.github.com/2401574 might be relevant to this, too.
Running into a somewhat if not exact same crash -- see attached log -- Nightly (04/16), Galaxy Nexus (Android 4.0.4)
Setting target milestone for inbound.
Target Milestone: --- → Firefox 14
(In reply to Richard Newman [:rnewman] from comment #7) > I have temporarily disabled the password sync stage until this bug is > resolved. > > https://hg.mozilla.org/integration/mozilla-inbound/rev/bbfe59782089 This was re-enabled by: changeset: 91647:190a237b1456 user: Nick Alexander <firstname.lastname@example.org> date: Fri Apr 13 19:41:40 2012 -0700 summary: Bug 739629 - Expose safe account creation API for profile migration. r=rnewman
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
verified with m-c nightly build of 20120418
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.