941892 - crash in mozalloc_abort(char const* const) | NS_DebugBreak | mozilla::plugins::PluginModuleParent::StreamCast(_NPP*, _NPStream*)

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[juan becerra [:juanb]]

This bug was filed from the Socorro interface and is 
report bp-cf66b499-4103-49bb-a5e0-a59ff2131121.
=============================================================

This started to appear in the explosive reports for Firefox 26 beta on around 11/20/2013, but it is not in the list of top crashers. Most of the reports are happening in 25.0.1 and Windows 7, and many of the comments mention it happening while playing videos.

Comment 1

[Benjamin Smedberg]

See also: bug 556643, bug 804368, bug 802355.

Do we have Flash versions for these crashes?

Comment 2

[Benjamin Smedberg]

Need URLs; Tracy is that something you can do? Unlikely to be a recent Mozilla regression, but something triggered either by a Flash update or a website change.

Comment 3

[Tracy Walker [:tracy]]

I can... here are the top bunch (less a few inappropriate youtube videos):

81 	https://www.facebook.com/
69 	about:blank
15 	http://video.bobdylan.com/desktop.html
8 	about:newtab
7 	https://maps.google.co.uk/maps?hl=en
7 	http://www.torontosun.com/
6 	http://www.colbertnation.com/full-episodes/wed-november-20-2013-m-i-a-
6 	http://www.reddit.com/
6 	http://www.thedailyshow.com/full-episodes/wed-november-20-2013-tom-brokaw
6 	http://www.colbertnation.com/full-episodes/tue-november-19-2013-rick-santorum
6 	http://www.ad.nl/

Comment 4

[Jaryth]

Experiencing this issue myself. Have had no issues since 25.0.1 was released until today, I have experienced 9 crashes in the span of 4 hours today, and has changed that I know of.
Started on Flash 11.8.800.174 when the issue started, tried updating to 11.9.900.152 and the issue continued.

All of the crashes happened on Youtube. Crash report links if anyone would find them helpful:
bp-b57693b8-688a-45b0-8202-f32c02131122
bp-d723cde2-8423-4840-b1b8-2a9c42131122
bp-08c1cdf9-91d7-4ca3-a6ba-6edb12131122
bp-16b0217e-3ceb-4f4b-ae0f-275c72131122
bp-bd26a47f-930a-45f9-8684-7a0d42131122
bp-477ec963-41f0-488d-bb37-47cca2131122
1811d7a1-4ee6-4831-9968-2440fd8e99f9	 
bp-4b5c6f2a-591e-4aaa-9589-9f18c2131122
bp-3053cebe-ef09-44a3-9ffe-c99512131122
bp-9243a08b-8cec-4106-a383-8c7202131122

Hopefully this information proves somewhat useful.

Comment 5

[Robert Kaiser]

This is shooting into high topcrash ranges across all channels in yesterday's data.

Comment 6

[juan becerra [:juanb]]

So far I have not been able to reproduce this on Win 7 VMs, with the information I've gathered in the crash report comments, but I have emailed about a dozen users affected, and I've requested more information.

Comment 7

[juan becerra [:juanb]]

I don't know if any Microsoft Windows updates come into play, but from the following page, it looks like their last patch Tuesday was on 11/12, about a week+ ago: https://technet.microsoft.com/en-us/security/bulletin/ms13-nov

Comment 8

[Jaryth]

As an effected user, I have not installed any Windows Updates in well over a month (something I should really get on). Microsoft Office 2013 released an update that I installed on 11/22 and the most recent application I've installed was an update for Reflector (http://www.airsquirrels.com/reflector/) that was released 11/07.

Comment 9

[Benjamin Smedberg]

Another likely possibility is that an addon update may have caused this, especially because I see stream-listener-tee and JS on the stack. Tracy or Juan can you check the addon correlations to see if something shows up high on the list?

Comment 10

[Tracy Walker [:tracy]]

I actually tried to dig this up earlier today.  But correlations for the stack aren't working. so I went to the raw data. There I could not find this signature.  Other variations of mozalloc_abort(char const* const) | NS_DebugBreak | * are there, but not this one. 

KaiRo any ideas I might be able to look into?

Comment 11

[juan becerra [:juanb]]

I went through several reports, and then I looked in the extensions tab for those individual reports. There are three or add-ons in common with the reports I looked at, something called Hola (which lets you play videos where licensing doesn't allow you to), Adblock Plus, and Video Download Helper.

Several comments called out the Hola app explicitly. I tried installing it from their site (the one in AMO is not compatible), but the latest version is from today and I haven't been able to reproduce the problem with that and youtube videos yet.

Comment 12

[John Schoenick [:johns]]

The hola addon is present in all the crashes I checked. It looks like a JS-Implemented stream listener is passing the a stream to the wrong plugin listener, causing a runtime abort. However, it seems like we could return an error in nsPluginStreamListener if it receives an unknown stream and avoid the need to abort in the lower level code here.

Comment 13

[John Schoenick [:johns]]

Attachment: Take an early return if nsPluginStreamListenerPeer gets passed an unknown stream

@bsmedberg - Are you familiar with the code here? I think this is a good change,
but this might be one of those asserts-constantly-but-is-never-noticed cases, so
I'm not sure how safe taking this on branches will be.

Comment 14

[Kevin Brosnan [Ex-Mozilla]]

A user on Reddit says that this happens every time he fast forwards. http://www.reddit.com/r/firefox/comments/1r7arb/is_youtube_crashing_firefox_for_anyone_else/cdkfy8m

Comment 15

[Benjamin Smedberg]

Comment on attachment 8337052 [details] [diff] [review]
Take an early return if nsPluginStreamListenerPeer gets passed an unknown stream

I don't know whether our tests might hit this or not. r=me to land with the MOZ_ASSERT or keeping the less fatal NS_ASSERTION if a try run shows orange.

Comment 16

[John Schoenick [:johns]]

I misread comment 0, this isn't a recent regression

Comment 17

[John Schoenick [:johns]]

Try:
https://tbpl.mozilla.org/?tree=Try&rev=492b1621d518

Comment 18

[help_be]

Hola latest version solved this crash. The version was submitted last week and we are waiting for Mozilla to approve the version.

Comment 19

[Robert Kaiser]

It looks to me like this crash is fixed by the blocklisting in bug 942935 and probably also a new version from Hola.

Thanks everyone participating for working on fixing this and making our users' lives better!

Comment 20

[Robert Kaiser]

Oh, wait, I forgot there's a patch on here from our side, so let's keep it open until that lands as well.

Comment 21

[John Schoenick [:johns]]

Comment on attachment 8337052 [details] [diff] [review]
Take an early return if nsPluginStreamListenerPeer gets passed an unknown stream

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9ff42335f8a

Comment 22

[John Schoenick [:johns]]

this is invalid behavior to begin with, so figuring out a test isn't really worthwhile. Since Hola was updated and old versions blocked, there's probably little need to uplift.

Comment 23

[Carsten Book [:Tomcat]]

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

Comment 24

[Tracy Walker [:tracy]]

(In reply to John Schoenick [:johns] from comment #22)
> this is invalid behavior to begin with, so figuring out a test isn't really
> worthwhile. Since Hola was updated and old versions blocked, there's
> probably little need to uplift.

Is there a possibility this could be trigger by other addons?  Requesting tracking all over to get decision from RelMan to wontfix or not.

I'll keep an eye on crash stats to verify this drops off topcrash lists.  I won't be able to tell whether it was this fix, the blocklist or the addon update that actually resolved the issue. But with the three combined, we should be good.

Comment 25

[Benjamin Smedberg]

Yes, we have seen other addons trigger this in the past. This is a low-risk fix but not something we need to take on ESR or 26. Worth uplifting to 27 probably.

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

Renom if this does get into topcrash territory, otherwise risk and timing make comment 25 the best path here.

Comment 27

[John Schoenick [:johns]]

Comment on attachment 8337052 [details] [diff] [review]
Take an early return if nsPluginStreamListenerPeer gets passed an unknown stream

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A

User impact if declined:
Mis-handling stream listeners in addons can trigger crash.

Testing completed (on m-c, etc.): On m-c

Risk to taking this patch (and alternatives if risky): 
Low risk, code hitting this path would almost always crash without the patch.

String or IDL/UUID changes made by this patch: None

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e34cda9a4521

Comment 29

[Tracy Walker [:tracy]]

Verified via crash stats that this signature is no longer occurring with recent builds on both Aurora(27) and Nighty(28).

Comment 30

[Robert Kaiser]

Looking at https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozalloc_abort%28char+const*+const%29+|+NS_DebugBreak+|+mozilla%3A%3Aplugins%3A%3APluginModuleParent%3A%3AStreamCast%28_NPP*%2C+_NPStream*%29 it doesn't look like it ever actually went away. Did we reintroduce it?

Comment 31

[John Schoenick [:johns]]

Indeed, the spike went away, but the patch here did not stop the issue -- we're still somehow passing the wrong stream to the wrong plugin instance :-/

Comment 32

[John Schoenick [:johns]]

Some plugin is still intercepting the plugin channel, then passing OnDataAvailable to the wrong plugin instance. There's no correlation data on the bug, but all the crashes I looked at had "NoTrace" installed: http://isis.dia.unisa.it/projects/NoTrace/

Comment 33

[Jorge Villalobos [:jorgev] (he/him)]

The developers have been contacted through AMO.

Comment 34

[Mike Kaply [:mkaply]]

I got this without Hola (albeit with a different add-on, not sure if related).

https://crash-stats.mozilla.com/report/index/71970233-d1dc-497a-9895-378772150226

I can recreate reasonably consistenly on Windows if anyone wants me to look at something. About to build debug to see if I can catch there.

Comment 35

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm marking this bug as FIXED per comment #23 and also bug crashlog signature didn't appear from a long time (over half year) [except some obsolete <29 versions, no crashes starting since 29 version].