Closed Bug 1652409 Opened 1 year ago Closed 1 year ago

Crash in [@ mozilla::dom::indexedDB::(anonymous namespace)::ReadCompressedNumber]

Categories

(Core :: Storage: IndexedDB, defect, P2)

Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox78 --- unaffected
firefox79 --- fixed
firefox80 --- fixed

People

(Reporter: gsvelto, Assigned: sg)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-6f44dad1-9131-475d-8538-71cd60200713.

Top 10 frames of crashing thread:

0 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:662
1 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:799
2 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:894
3 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:25018
4 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:22717
5 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:22883
6 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234
7 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:513
8 XUL mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:12991
9 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1234

This appeared in nightly with buildid 20200705095345 but even earlier in the beta channel with buildid 20200703001609. The raw crash reason is MOZ_RELEASE_ASSERT(span_ && (index_ + n) >= 0 && (index_ + n) <= span_->Length()). Somehow it only appears to affect macOS users.

Assignee: nobody → sgiesecke

The anonymous namespace part of the signature is different between OS X and Windows. I added the Windows signature, there are crashes as well.

Crash Signature: [@ mozilla::dom::indexedDB::(anonymous namespace)::ReadCompressedNumber] → [@ mozilla::dom::indexedDB::(anonymous namespace)::ReadCompressedNumber] [@ mozilla::dom::indexedDB::`anonymous namespace'::ReadCompressedNumber]

On Windows, the first crash with this signature seems to be from build id 20200629224419

Given the low volume of crash reports, even though that landed almost a week earlier, this is most likely to be caused by https://hg.mozilla.org/mozilla-central/rev/abd728a13d825387c87b1488610aa6d4759a6c13 from Bug 1623278

The particular assertion was introduced by the mentioned patch.

This might be a real regression, a change in signatures or some existing issue that wasn't caught before by an assertion.

OS: macOS → All

This seems to be the case that is already referenced by an XXX comment here: https://searchfox.org/mozilla-central/rev/af5cff556a9482d7aa2267a82f2ccfaf18e797e9/dom/indexedDB/ActorsParent.cpp#659

In my understanding, this is an existing issue that wasn't caught before by an assertion, but caused some error elsewhere before (not sure if a run-time error, a release assertion or just UB).

Jan, WDYT, should we change this to return an error in that case? Probably NS_ERROR_FILE_CORRUPTED?

Flags: needinfo?(jvarga)

I guess that's what was ultimately happening before e.g. here: https://searchfox.org/mozilla-central/rev/cc4a5ebe14c5f8b54f744164e53805eec26aab07/dom/indexedDB/ActorsParent.cpp#825-831

So maybe one can view this as a regression of Bug 1623278, which implicitly promoted this from a runtime error to a release assertion. I'll prepare a patch.

Regressed by: 1623278

Yeah, that makes sense, we probably don't have any other options other than returning NS_ERROR_FILE_CORRUPTED.

Flags: needinfo?(jvarga)
Severity: -- → S3
Priority: -- → P2
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/223990c25952
Ensure that a corrupted file encountered during ReadCompressedNumber does not cause an assertion failure. r=dom-workers-and-storage-reviewers,janv
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

Is there a user impact here which justifies Beta backport consideration? Please nominate for approval if yes :)

Comment on attachment 9163165 [details]
Bug 1652409 - Ensure that a corrupted file encountered during ReadCompressedNumber does not cause an assertion failure. r=#dom-workers-and-storage

Beta/Release Uplift Approval Request

  • User impact if declined: This only impacts cases of corrupted storage, but it will prevent the parent process from crashing safely in this case, and instead let an error be reported to the content process.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: We don't have STR. (It would be good to have automated tests in general for cases of corrupted storage to better catch things like this.)
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Changes to error handling only to prevent a release assertion to fail.
  • String changes made/needed:
Flags: needinfo?(sgiesecke)
Attachment #9163165 - Flags: approval-mozilla-beta?

Comment on attachment 9163165 [details]
Bug 1652409 - Ensure that a corrupted file encountered during ReadCompressedNumber does not cause an assertion failure. r=#dom-workers-and-storage

Approved for 79.0b9.

Attachment #9163165 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.