Closed Bug 1306432 Opened 8 years ago Closed 8 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: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.