933156 - mozilla::dom::PannerNodeEngine::GainStereoToStereo reads out of bounds

Status: VERIFIED FIXED

(Core :: Web Audio, defect)

Description

[Jesse Ruderman]

Attachment: testcase (takes 4 seconds)

Assertion failure: i < Length() (invalid array index), at nsTArray.h:880

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Maire Reavy [:mreavy]]

Karl-- Can you look at this?

Comment 3

[Karl Tomlinson (:karlt)]

This is a regression from https://hg.mozilla.org/mozilla-central/rev/578c80c21547.

It could make two 512-byte blocks of memory accessible to content.  The addresses of these blocks are from the two pointer-sized words after nsTArrayHeader::sEmptyHdr.  sEmptyHdr is not declared const, so there is the chance of these pointers being influenced by content but that seems unlikely.  Considering this a mitigating circumstance, marking sec-moderate.

Comment 4

[Karl Tomlinson (:karlt)]

> It could make two 512-byte blocks of memory accessible to content.

read-accessible, that is.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: don't send null input to EqualPowerPanningFunction

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 825641 [details] [diff] [review]
don't send null input to EqualPowerPanningFunction

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 898291
User impact if declined: security risk
Testing completed (on m-c, etc.): testcase
Risk to taking this patch (and alternatives if risky):
very low.  small patch + infuence
String or IDL/UUID changes made by this patch: none

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 825641 [details] [diff] [review]
don't send null input to EqualPowerPanningFunction

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: bug 898291 shouldn't land on Beta without this
Testing completed (on m-c, etc.): testcase
Risk to taking this patch (and alternatives if risky):
very low.  small patch + small influence
String or IDL/UUID changes made by this patch: none

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 825641 [details] [diff] [review]
don't send null input to EqualPowerPanningFunction

This is marked as unaffected on FF26 - so no need for mozilla-beta approval here.

Comment 9

[Maire Reavy [:mreavy]]

Comment on attachment 825641 [details] [diff] [review]
don't send null input to EqualPowerPanningFunction

We want beta approval for this so that we can land bug 898291 (a noticeable performance win) on Firefox 26.  See Comment 7: "User impact if declined: bug 898291 shouldn't land on Beta without this."  Bug 898291 was just approved for Beta.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e6063aa9b77

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #10)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/2e6063aa9b77

Wrong cset. This is the right one:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c42c1d8bd703

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7b2d872b4496

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c42c1d8bd703

Comment 14

[David Chan [:dchan]]

b2g-v1.2 shouldn't be affected by this since fx26 isn't. However I'm not sure given the regression was landed the Friday before merge.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/112e82a7612c

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/112e82a7612c

Comment 17

[Ioana (away)]

I tried to reproduce this issue on a 10/31 Fx26 debug build and on a 10/26 Fx26 ASAN build to see if I can verify it locally. I waited several minutes (for those 4 seconds) and tried this multiple times but it didn't reproduce once.

I worked on Ubuntu 12.10 64bit.

Any ideas what else I could do to reproduce this bug?

Comment 18

[Karl Tomlinson (:karlt)]

Landed a slightly reduced test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9768d081b7d8

Comment 19

[Jesse Ruderman]

Ioana, this bug didn't affect any Firefox 26 builds because the regressing changeset was held back on that branch until this bug's patch had approval.  See comment 7 through comment 9.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9768d081b7d8

Comment 21

[Ioana (away)]

(In reply to Jesse Ruderman from comment #19)
> Ioana, this bug didn't affect any Firefox 26 builds because the regressing
> changeset was held back on that branch until this bug's patch had approval. 
> See comment 7 through comment 9.

Sorry, the status-firefox26 set as fixed threw me off.

I reproduced the bug on a 10/31 Nightly ASAN and verified it on current ASAN builds: 11/11 Nightly, 11/11 Aurora, 11/10 Beta. Tested on Ubuntu 12.10 x86_x64.