Closed Bug 1306432 Opened 9 years ago Closed 9 years ago

Null deref crash in Stagefright-Rust checking code in GetTrackInfo

Categories

(Core :: Audio/Video: Playback, defect, P1)

Product:
Core
Component:
Audio/Video: Playback
Platform:
Unspecified
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- disabled

People

(Reporter: mccr8, Assigned: kinetik)

References

Details

(Keywords: crash)

Crash Data

This bug was filed from the Socorro interface and is report bp-8cf8894a-1f32-4226-9df1-241242160929. ============================================================= It looks like infoRust might be null. This is non-release code hidden behind a pref so it doesn't matter from a stability perspective. It seems odd to me that this code is compiled in non-debug builds, but it is using MOZ_ASSERT instead of MOZ_RELEASE_ASSERT. I think either this code should all be debug-only, or you want MOZ_RELEASE_ASSERT. Then you'd hit a release assert back on line 285 instead of this null deref.
status-firefox51: --- → unaffected
status-firefox52: affected → disabled
Flags: needinfo?(kinetik)
(In reply to Andrew McCreight [:mccr8] from comment #0) > It looks like infoRust might be null. This is non-release code hidden behind > a pref so it doesn't matter from a stability perspective. That'll be me crashing. The fix for this has been pushed upstream: https://github.com/mozilla/mp4parse-rust/commit/c08a02ddc1046837f16cdaeb7e365f118476b339 We need to update the Gecko version to pick up the fix, this bug can track that. > It seems odd to me that this code is compiled in non-debug builds, but it is > using MOZ_ASSERT instead of MOZ_RELEASE_ASSERT. I think either this code > should all be debug-only, or you want MOZ_RELEASE_ASSERT. Then you'd hit a > release assert back on line 285 instead of this null deref. Yeah, that was an oversight. It was changed to MOZ_RELEASE_ASSERT in bug 1305596.
Flags: needinfo?(kinetik)
(In reply to Matthew Gregan [:kinetik] from comment #1) > That'll be me crashing. I figured as much. > The fix for this has been pushed upstream: > https://github.com/mozilla/mp4parse-rust/commit/ > c08a02ddc1046837f16cdaeb7e365f118476b339 Great. I guess this can be closed. > Yeah, that was an oversight. It was changed to MOZ_RELEASE_ASSERT in bug > 1305596. Ah, sorry, I should have looked at what the code was on trunk.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Another couple of crashes with this signature, but now on the mRate == mRate line. That's https://github.com/mozilla/mp4parse-rust/issues/30.
Fixed by bug 1303888 pulling the latest fixes into Gecko.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.