Last Comment Bug 398271 - Insert with no `nodeset` declaration causes crash
: Insert with no `nodeset` declaration causes crash
Status: RESOLVED FIXED
: fixed1.8.1.12
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: John L. Clark
:
:
Mentors:
Depends on: 391586
Blocks: 410239
  Show dependency treegraph
 
Reported: 2007-10-02 07:56 PDT by John L. Clark
Modified: 2016-07-15 14:46 PDT (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to test the pointer before dereferencing it (876 bytes, patch)
2007-10-02 07:59 PDT, John L. Clark
bugs: review+
Details | Diff | Splinter Review

Description John L. Clark 2007-10-02 07:56:47 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007100210 Minefield/3.0a9pre
Build Identifier: CVS HEAD, 2007-10-02T10:30:00-04:00

In some cases, inserting a node with no `nodeset` attribute specified will cause the browser to crash with the following (partial) backtrace:

0xffffe410 in __kernel_vsyscall ()
(gdb) bt
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb733de66 in nanosleep () from /lib/tls/i686/cmov/libc.so.6
#2  0xb733dc8f in sleep () from /lib/tls/i686/cmov/libc.so.6
#3  0xb7ec26d9 in ah_crap_handler (signum=11) at nsSigHandlers.cpp:149
#4  0xb7edb854 in nsProfileLock::FatalSignalHandler (signo=11) at nsProfileLock.cpp:210
#5  <signal handler called>
#6  0xb4ad27c6 in nsXFormsInsertDeleteElement::HandleAction (this=0x9599ba8, aEvent=0x9ae5d78,
    aParentAction=0x9599a94)
    at /home/john/development/mozilla/extensions/xforms/nsXFormsInsertDeleteElement.cpp:243
#7  0xb7d16c41 in NS_InvokeByIndex_P ()
    at /home/john/development/mozilla/xpcom/reflect/xptinfo/src/xptiInterfaceInfo.cpp:73

Reproducible: Always
Comment 1 John L. Clark 2007-10-02 07:59:21 PDT
Created attachment 283196 [details] [diff] [review]
Patch to test the pointer before dereferencing it

The `nodeset` variable is null if no `nodeset` attribute is specified, so we just bypass the assignment of `nodesetSize` (instead allowing it to remain 0).
Comment 2 Olli Pettay [:smaug] (reviewing overload) 2007-10-02 08:54:09 PDT
This is probably a regressions from bug 391586.
Merle can hopefully do the first review (use the 'Details' link on the 
attachment to set 'review ? msterlin@us.ibm.com').
Btw, in the future, use also -p option of cvs diff.
Comment 3 Merle Sterling 2007-10-02 11:14:54 PDT
I am not authorized to edit the bug to add myself as a reviewer.

The error most likely is a regression from bug 391586 because I had to rearrange a lot of the code to get the in-scope evaluation context correct and apparently left out the check for a non-null nodeset. The patch is fine, so r=me.

 

Comment 4 Olli Pettay [:smaug] (reviewing overload) 2007-10-03 02:09:32 PDT
Checked in
Comment 5 aaronr 2008-01-08 19:23:08 PST
checked into 1.8 branch via bug 410239.

Note You need to log in before you can comment on or make changes to this bug.