Closed Bug 1009036 Opened 10 years ago Closed 10 years ago

Use-after-poison of nsStyleContext with bidi, convertPointFromNode

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla32
Tracking Flags:
Tracking Status
firefox31 --- wontfix
firefox32 --- verified
firefox-esr24 --- wontfix
firefox-esr31 --- wontfix
b2g-v1.4 --- unaffected

People

(Reporter: jruderman, Assigned: MatsPalmgren_bugz)

References

Details

(4 keywords, Whiteboard: [adv-main32-])

Attachments

(5 files, 1 obsolete file)

testcase (crashes Firefox)
289 bytes, text/html
Details
stack (lldb)
10.59 KB, text/plain
Details
stack (ASan)
17.11 KB, text/plain
Details
stack for frame destruction
11.13 KB, text/plain
Details
fix
2.26 KB, patch
roc
: review+
Details | Diff | Splinter Review 
      No description provided.
Attached file stack (lldb) — 

    
    

    
  

      
      
      
      

        Attached file
          
        stack (ASan)
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        wip (obsolete)
        — 
          — Splinter Review
      

    


  
I think what happens is that the second GetFirstNonAnonymousFrameForGeometryNode
call (for the text node) leads to EnsureFrameForTextNode which inserts a new
child frame into 'fromFrame' which causes frames to be reconstructed for some
reason and thus 'fromFrame' points to a destroyed frame.  Checking the if it's
still alive should be good enough I think.  (iirc, we discussed this scenario
during review but dismissed it since the second Flush_Layout couldn't possibly
do anything after we had already flushed in the first call).

http://mxr.mozilla.org/mozilla-central/source/layout/base/GeometryUtils.cpp#33
Assignee: nobody → matspal

    
    

    
  


  
And the "for some reason" is we destroy the whole frame tree for performance! :-)
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#7806

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix
        — 
          — Splinter Review
      

    


  
It looks like GetBoxQuads might have same problem, so I fixed it too.

        Attachment #8421142 -
        Attachment is obsolete: true

        Attachment #8421189 -
        Flags: review?(roc)

    
  
Keywords: csectype-framepoisoning, 
          
            sec-other
OS: Mac OS X → All
Hardware: x86_64 → All

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/437e6e7eba92
Status: NEW → RESOLVED
Closed: 10 years ago

          status-firefox32:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla32

    
    

    
  
Confirmed crash in Fx32 2014-04-29.
Verified fixed in Fx32 2014-08-22.
Status: RESOLVED → VERIFIED

          status-firefox32:
          fixed → verified

          status-firefox-esr31:
          --- → wontfix
Whiteboard: [adv-main32-]

    
  
Group: core-security → core-security-release
Group: core-security-release

    
  
Flags: in-testsuite? → in-testsuite+

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: