60844 - N601 Crash (Linux) #7: Mozilla crashes when using plugger from redhat. [@ nsPluginTag::nsPluginTag]

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Knut J]

I have Redhat linux 7 with Netscape 6 and latest Mozilla night build. Both of
them can not use realplayer eventhough I have copy rpnp.so and raclass into
plugins folder. PLugger cause booth browsers to crash.

Comment 1

[Boris Zbarsky [:bzbarsky]]

See bug 56464 on the realplayer issue.

As for plugger, I see mozilla registering the plugin but getting a SIGSEGV
immediately afterward (linux CVS build from source dated 11/20/2000 16:39 PST,
plugger 3.2).  Changing summary to refer only to plugger, confirming bug.  Stack
trace from segfault:

#0  0x4187901e in nsPluginTag::nsPluginTag (this=0x81a7628, aPluginInfo=0xbffff24c)
    at nsPluginHostImpl.cpp:463
        result = 0x81a9648 "audio/x-mpegurl"
        str = 0x100 <Address 0x100 out of bounds>
        i = 40
#1  0x41880829 in nsPluginHostImpl::ScanPluginsDirectory (this=0x819afd0,
    pluginsDir=@0xbffff2e8, compManager=0x8066118, layoutPath=0x819b058,
    checkForUnwantedPlugins=0, checkForDups=0) at nsPluginHostImpl.cpp:3000
        pluginFile = {<nsFileSpec> = {mPath = {mData = 0x81a4e28}, mError = 0,
    _vptr. = 0x4189dc84 <nsPluginFile virtual table>}, pLibrary = 0x81a7168}
        pluginLibrary = (struct PRLibrary *) 0x81a7168
        info = {fPluginInfoSize = 32, fName = 0x8079628 "Plugger 3.2",
  fDescription = 0x81a7680 "<img width=40 height=40 border=0 align=left
src=http://fredrik.hubbe.net/plugger/logo.gif><a
href=http://fredrik.hubbe.net/plugger.html>Plugger</a> version 3.2, written by
<a href=http://fredrik.hubbe"..., fVariantCount = 41,
  fMimeTypeArray = 0x81a96f8, fMimeDescriptionArray = 0x81a97a0,
  fExtensionArray = 0x81a9848, fFileName = 0x0}
        res = 0
        pluginTag = (nsPluginTag *) 0xbffff26c
        bAddIt = 1
        iter = {mCurrent = {mPath = {mData = 0x81a4e28}, mError = 0,
    _vptr. = 0x4015ac1c <nsFileSpec virtual table>}, mExists = 1,
mResoveSymLinks = 1,
  mStarting = {mPath = {mData = 0x819d8d8}, mError = 0,
    _vptr. = 0x4015ac1c <nsFileSpec virtual table>}, mDir = 0x81a4f38,
  _vptr. = 0x4015abec <nsDirectoryIterator virtual table>}
#2  0x41880c3a in nsPluginHostImpl::LoadPlugins (this=0x819afd0)
    at nsPluginHostImpl.cpp:3074
        lpath = {mRawPtr = 0x819b058}
        path = {mRawPtr = 0x819b058}
        isLayoutPath = 1
        rv = 2152792067
        compManager = {mRawPtr = 0x8066118}
        pluginsDir = {<nsFileSpec> = {mPath = {mData = 0x81a4d18}, mError = 0,
    _vptr. = 0x4189dc90 <nsPluginsDir virtual table>}, <No data fields>}
#3  0x4188047c in nsPluginHostImpl::GetPluginFactory (this=0x819afd0,
    aMimeType=0x4185dce3 "application/x-java-vm", aPlugin=0xbffff388)
    at nsPluginHostImpl.cpp:2846
        this = (nsPluginHostImpl *) 0x819afd0
        rv = 3221222300
        pluginTag = (nsPluginTag *) 0x40161398
#4  0x41850785 in nsJVMManager::StartupJVM (this=0x819cc20) at nsJVMManager.cpp:594
        start = 4197943586
        err = 0
        pluginHost = {mRawPtr = 0x819afd4}
        pluginFactory = (nsIPlugin *) 0x0
        rslt = 0
        end = 0
        d = 135960136
#5  0x41850e4d in nsJVMManager::MaybeStartupLiveConnect (this=0x819cc20)
    at nsJVMManager.cpp:783
        this = (nsJVMManager *) 0x819cc20
#6  0x41851a18 in nsJVMManager::StartupLiveConnect (this=0x819cc20,
runtime=0x8107dc8,
    outStarted=@0xbffff42c) at nsJVMManager.h:128
        outStarted = (PRBool &) @0xbffff42c: 0
#7  0x40489c55 in nsJSEnvironment::nsJSEnvironment (this=0x819c520)
    at nsJSEnvironment.cpp:1472
        started = 0
        this = (nsIObserver *) 0x819c520
        rv = 0
        observerService = {mRawPtr = 0x80a5e10}
        manager = {mRawPtr = 0x819cc28}
#8  0x404896b9 in nsJSEnvironment::GetScriptingEnvironment () at
nsJSEnvironment.cpp:1417
No locals.
#9  0x40489ff2 in NS_CreateScriptContext (aGlobal=0x81477d8, aContext=0x81454e8)
    at nsJSEnvironment.cpp:1512
        aGlobal = (nsIScriptGlobalObject *) 0x81477d8
        aContext = (nsIScriptContext **) 0x0
        environment = (nsJSEnvironment *) 0x0
        scriptContext = (nsIScriptContext *) 0xbffff54c
#10 0x40e15069 in nsDocShell::EnsureScriptEnvironment (this=0x8145438)
    at nsDocShell.cpp:4306
No locals.
#11 0x40e16628 in nsWebShell::GetInterface (this=0x8145438, aIID=@0x40603890,
    aInstancePtr=0xbffff5d8) at nsWebShell.cpp:330
        this = (nsWebShell *) 0x8145438
        aIID = (nsIID &) @0x40603890: {m0 = 2626754656, m1 = 32217, m2 = 4564,
  m3 = "\232\203\000\000dest"}
        rv = 0
#12 0x40103f20 in nsGetInterface::operator() (this=0xbffff630, aIID=@0x40603890,
    aInstancePtr=0xbffff5d8) at nsIInterfaceRequestor.cpp:37
        factoryPtr = {mRawPtr = 0x814545c}
        status = 0
#13 0x405ee29a in nsCOMPtr<nsIDOMWindowInternal>::assign_from_helper
(this=0xbffff62c,
    helper=@0xbffff630, aIID=@0x40603890) at ../../../dist/include/nsCOMPtr.h:856
        this = (nsCOMPtr<nsIDOMWindowInternal> *) 0xbffff62c
        helper = (nsCOMPtr_helper &) @0x81a9648: {_vptr. = 0x69647561}
        newRawPtr = (nsIDOMWindowInternal *) 0x0
#14 0x405d768a in nsAppShellService::GetHiddenWindowAndJSContext (this=0x80ac700,
    aWindow=0xbffff678, aJSContext=0xbffff670) at
../../../dist/include/nsCOMPtr.h:552
        this = (nsCOMPtr<nsIDOMWindowInternal> *) 0xbffff62c
        docShell = {mRawPtr = 0x8145438}
        hiddenDOMWindow = {mRawPtr = 0x0}
        sgo = {mRawPtr = 0x4015e2f4}
        scriptContext = {mRawPtr = 0x8145438}
        jsContext = (struct JSContext *) 0xbffff630
        this = (nsAppShellService *) 0x81a9648
        rv = 0
#15 0x405d4861 in nsAppShellService::SetXPConnectSafeContext (this=0x80ac700)
    at nsAppShellService.cpp:191
        rv = 0
        xpc = {mRawPtr = 0x81468b8}
        junk = {mRawPtr = 0x0}
        cx = (JSContext *) 0x0
#16 0x405d54f1 in nsAppShellService::CreateHiddenWindow (this=0x80ac700)
    at nsAppShellService.cpp:247
        newWindow = {mRawPtr = 0x8155d58}
        rv = 0
        hiddenWindowURL = 0x405fafa3 "about:blank"
        url = {mRawPtr = 0x8124b60}
#17 0x80520f9 in main1 (argc=1, argv=0xbffff8a4, nativeApp=0x0) at
nsAppRunner.cpp:988
        rv = 0
        eventQService = {mRawPtr = 0x808e570}
        obsService = {mRawPtr = 0x80a5e10}
        needAutoreg = 0
        cmdLineArgs = {mRawPtr = 0x80ac5c0}
        appShell = {mRawPtr = 0x80ac700}
        walletService = {mRawPtr = 0x4015ce58}
#18 0x8052d26 in main (argc=1, argv=0xbffff8a4) at nsAppRunner.cpp:1255
        argv = (char **) 0xbffff8a4
        nativeApp = (nsINativeAppSupport *) 0x0
        rv = 0
        splash = (nsISplashScreen *) 0x0
        dosplash = 0
        remoterv = 0
        argused = 0
        mainResult = 0
#19 0x403019cb in __libc_start_main (main=0x8052ba0 <main>, argc=1,
argv=0xbffff8a4,
    init=0x804c244 <_init>, fini=0x805edcc <_fini>, rtld_fini=0x4000ae60
<_dl_fini>,
    stack_end=0xbffff89c) at ../sysdeps/generic/libc-start.c:92
        argv = (char **) 0xbffff8a4
        rtld_fini = (void (*)(void)) 0x4000ae60 <_dl_fini>
        stack_end = (void *) 0x81a9648

Comment 2

[Jay Patel [:jay]]

Adding topcrash keyword and [@ nsPluginTag::nsPluginTag] for tracking, this is
the #7 topcrash for RTM on Linux.  Below is the stack trace and some user info
from talkback:

nsPluginTag::nsPluginTag()
	 nsPluginHostImpl::ScanPluginsDirectory()
	 nsPluginHostImpl::LoadPlugins()
	 nsPluginHostImpl::GetPluginFactory()
	 nsJVMManager::StartupJVM()
	 nsJVMManager::MaybeStartupLiveConnect()
	 nsJVMManager::StartupLiveConnect()
	 nsJSEnvironment::nsJSEnvironment()
	 nsJSEnvironment::GetScriptingEnvironment()
	 NS_CreateScriptContext()
	 nsDocShell::EnsureScriptEnvironment()
	 nsWebShell::GetInterface()
	 operator []()
	 nsCOMPtr_base::assign_from_helper()
	 nsAppShellService::GetHiddenWindowAndJSContext()
	 nsAppShellService::SetXPConnectSafeContext()
	 nsAppShellService::CreateHiddenWindow()
	 main1()
	 main()
	 libc.so.6 + 0x18cbe (0x4027ccbe)

URL:(23126401) www.netscape.com
Comment: (23126401) trying to open netscape from Konquerer while on www.nick.com
URL:(23144908) www.purehit.com
Comment: (23126233) opening netscape from the start menu in KDe2
Comment: (23144908) Starting Netscape 6
Comment: (23155422) clicking on .netscape
Comment: (23130332) Did not want to reload Flash or RealPlayer from net just to
install pluginsfor NS6.  So I tried# cd /usr/lib/netscape/plugins (4.76)# find .
-print | cpio -pdl /usr/local/netscape/pluginscpio:
/usr/local/netscape/plugins/./libnullplugin.so notnsPluginTag::nsPluginTag()
a31e8351

Comment 3

[Gordon Tetlow]

I just downloaded Mozilla 0.7 and installed plugger 3.2. The plugin works
correctly, but whenever I want to check out the plugin's page (about:plugins),
Mozilla dies with the following error:
./run-mozilla.sh: line 72:  2297 Segmentation fault      $prog ${1+"$@"}

I hope this helps.
-gordon

Comment 4

[Michael O'Brien]

By doing an about:plugins and monitoring the text in the xterm, it seems like
mozilla is correctly parsing the /etc/pluggerrc file but segfaults when it hits
the end of the file.  This does not depend on the contents of the file.

Comment 5

[Mike Shaver (:shaver emeritus)]

Plugger hands back a MIME description that has a trailing separator (;, which is
then converted to | in SetMIMETypeSeparator.  This bug is caused by failure to
handle this gracefully.  I have a patch that lops off the empty MIME entry,
which fixes the bug for me, and will attach it.  av, can you review?

Comment 6

[Mike Shaver (:shaver emeritus)]

Attachment: Patch to handle empty trailing entry in MIME description

Comment 7

[Mike Shaver (:shaver emeritus)]

Attachment: MIME fix, new_str exorcism and warning cleanup

Comment 8

[Mike Shaver (:shaver emeritus)]

Finishing what I started: get rid of new_str, fix some mildly-spooky signedness
confusion, and move the static declarations of CALLBACK functions out of the
header, and into the .cpp, where they belong.

It now compiles for me with only one warning (--disable-pedantic): a
possibly-uninitialized warning from the AReadableString code, which is almost
certainly spurious.

Can I please get some review for this?  It's busting people every day, and the
fix is easy.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I assume the big block moved from ns4xPlugin.h to ns4xPlugin.cpp wasn't
modified.  I didn't look all that closely.

nsPluginHostImpl.cpp:
  In 3 places you used strdup instead of PL_strdup.

  For your change from SetLength(-1) to SetLength(0), shouldn't this
  allow you to remove the code in ns4xPluginStreamListener::OnStartBinding
  that corrects -1 to 0?  Perhaps a comment in nsIPluginStreamInfo.h
  about the meaning of 0 is in order (although it doesn't currently have any
  comments!)

  s/NULL/nsnull/

  I think (after hearing it from jag) do_GetService is now preferred to
  NS_WITH_SERVICE (but I don't really care), i.e.,
  nsCOMPtr<nsIPlugin>
    plugin(do_GetService("@mozilla.org/blackwood/pluglet-engine;1", &result));

nsPluginsDirUnix.cpp:

  Why fix only in nsPluginsDirUnix.cpp?  Wouldn't this bug affect
  all platforms?  (Roughly the same code seems to be duplicated in
  nsPluginsDir*.cpp for the other platforms.)  According to shaver,
  the only plugin he's found that gives this type of string is
  a Unix-only plugin (plugger).  However, maybe we should be a bit
  more crash-resistant on other platforms too?  (It's a pretty
  simple fix to copy over to the other files.)

  Perhaps it would also be nice to make the nsPluginsDirUnix.cpp
  version of CalulateVariantCount work like the windows one and
  add 1 internally, avoiding the additional variable where it's
  called?

Since this fixes a topcrash, r=dbaron if you fix the strdup->PL_strdup,
although I think it would be good to fix the other issues as well.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually there are 4 places where you used strdup instead of PL_strdup.

Comment 11

[Chris Waterson]

Couple comments:

1. I'm not convinced replacting `new_str()' with `strdup()' is safe because
   `new_str()' checks for a null argument. 

2. Your changes are mildly schizophrenic (you use both strdup() and
   PL_strdup(), pick one).

3. In this little part of Rome, C++ comments, please.

4. Why are you replacing nsPluginStreamInfo->SetLength(-1) with SetLength(0)?
   (I don't know if it's right or wrong, but vend me a clue.)

Comment 12

[Mike Shaver (:shaver emeritus)]

I thought I converted all my strdups to PL_strdups, but apparently not.  As
waterson points out, though, I may well have to revert to calling new_str, which
I will then fix to just call PL_strdup if the string isn't null.  I'm pretty
sure that _some_ of the cases can't be null, but safety first.

I didn't change the other platforms because I had no way of verifying their
behaviour, and the code is new enough to me that I didn't feel comfortable
operating blind.

I thought about the removal of that -1 -> 0 line, but I wasn't sure that there
were no Mac-only paths that could result in that.  I'm tempted to just back that
change out, because while I know that it's wrong to pass -1 as an unsigned
parameter, I'm not sure I can analyze the code well enough to make sure that
it's safe to pass 0.  And it (mostly) works now, right?  I'm going to back it out.

nsnull used, though most of the rest of the code seems to use NULL.

Waterson: in the part of Rome immediately preceding my comment is a C-style
comment.  You want I should convert them all?

New patch coming up.

Comment 13

[Mike Shaver (:shaver emeritus)]

Attachment: Incorporated dbaron's and waterson's comments

Comment 14

[Mike Shaver (:shaver emeritus)]

Attachment: one more try (PL_strdup checks for NULL)

Comment 15

[Mike Shaver (:shaver emeritus)]

Attachment: match PL_strdup with PL_strfree to keep Bruce at bay

Comment 16

[Chris Waterson]

sr=waterson

Comment 17

[Mike Shaver (:shaver emeritus)]

Damn, I'm an idiot.

PL_strdup returns |strdup("")| if passed a NULL pointer, which is not exactly
the same thing.  How much do we care?

(I wonder why it does that -- perhaps so that NULL return always signals an
error, but then I consider passing NULL to strdup to be an error too.)

Comment 18

[av (gone)]

Probably that's why new_str is used. It does the right thing in duplication 
nsPluginTag: if any member is null in the source it will be null in the target 
too. I could not get from what I read why you replaced it with strdup.

Comment 19

[Mike Shaver (:shaver emeritus)]

Right.  OK, so I'm going to update
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=23772 so that PL_strdup
is matched with PL_strfree, and attach it.  That will give us the current
dup-propagates-NULL behaviour, with uniform allocator behaviour.

Stay tuned.

Comment 20

[Mike Shaver (:shaver emeritus)]

Attachment: Minimal patch to replace the o'erreaching ones

Comment 21

[Mike Shaver (:shaver emeritus)]

My latest patch is _just_ a fix for the crash in question, plus a single use of
PL_strdup where I'm sure -- no, really -- it's correct.

Sorry for the flailing, please gimme the review I need to check it on in.

Comment 22

[Christopher Blizzard (:blizzard)]

r=blizzard

Comment 23

[av (gone)]

a=r=av

Comment 24

[Mike Shaver (:shaver emeritus)]

Brendan, I beseech you to sr= this patch.

Comment 25

[Brendan Eich [:brendan]]

Already done "verbally" (my words were transmitted) on IRC #mozilla, but for the
record, sr=[e-mail].

/be

Comment 26

[Peter Lubczynski]

Is this fixed/check-in?

Comment 27

[Mike Shaver (:shaver emeritus)]

Yeah, sorry.

Comment 28

[Curt Patrick (gone)]

updating summary to N601 as this is still a topcrasher in the N601 release.  
leaving resolved fixed...since this has been fixed in the trunk.

Comment 29

[shrirang khanzode]

*** Bug 70281 has been marked as a duplicate of this bug. ***

Comment 30

[shrirang khanzode]

marking verifd snce this was fixd on trunk.