Bug 112322 - Crash at RenderStyle::inheritFrom reported by fuzzer
Summary: Crash at RenderStyle::inheritFrom reported by fuzzer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Takashi Sakamoto
URL:
Keywords:
Depends on:
Blocks: 97279
  Show dependency tree / graph
 
Reported: 2013-03-13 23:54 PDT by Takashi Sakamoto
Modified: 2013-03-15 01:26 PDT (History)
7 users (show)

See Also:

Attachments
Patch (5.00 KB, patch)
2013-03-14 00:22 PDT, Takashi Sakamoto 		no flags Details | Formatted Diff | Diff
Patch (7.29 KB, patch)
2013-03-15 00:39 PDT, Takashi Sakamoto 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Takashi Sakamoto 2013-03-13 23:54:08 PDT 
Fuzzer	 Inferno_twister

Job Type	Linux_asan_chrome_mp
Crash type	UNKNOWN
Crash address	0x000000000030
Crash state	- crash stack -

WebCore::RenderStyle::inheritFrom
WebCore::StyleResolver::pseudoStyleForElement
WebCore::RenderObject::getUncachedPseudoStyle
Redzone	 32 bytes
Comment 1 Takashi Sakamoto 2013-03-13 23:54:32 PDT 
https://cluster-fuzz.appspot.com/testcase?key=159273524
Comment 2 Takashi Sakamoto 2013-03-14 00:22:24 PDT 
Created attachment 193080 [details]
Patch
Comment 3 Mike West 2013-03-14 02:42:56 PDT 
Comment on attachment 193080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review

> Source/WebCore/css/StyleResolver.cpp:1180
> +    RefPtr<RenderStyle> cloneForParent;

Nit: You're only using this in the else block below. It would likely be possible to define it there rather than here in the outer scope. Actually, I'm not sure it's necessary at all, as you could call `state.setParentStyle(RenderStyle::clone(state.style()))` directly with little loss in clarity.
Comment 4 Hajime Morrita 2013-03-14 02:51:04 PDT 
Comment on attachment 193080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review

> Source/WebCore/css/StyleResolver.cpp:1188
> +        state.setParentStyle(cloneForParent.get());

Why is this safe? It looks cloneForParent is gone after this function.
How about just make State::m_parentStyle RefPtr?
Comment 5 Takashi Sakamoto 2013-03-15 00:39:52 PDT 
Created attachment 193251 [details]
Patch
Comment 6 Takashi Sakamoto 2013-03-15 00:41:53 PDT 
Comment on attachment 193080 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193080&action=review

Thank you for reviewing.

>> Source/WebCore/css/StyleResolver.cpp:1180
>> +    RefPtr<RenderStyle> cloneForParent;
> 
> Nit: You're only using this in the else block below. It would likely be possible to define it there rather than here in the outer scope. Actually, I'm not sure it's necessary at all, as you could call `state.setParentStyle(RenderStyle::clone(state.style()))` directly with little loss in clarity.

I see. Done.

>> Source/WebCore/css/StyleResolver.cpp:1188
>> +        state.setParentStyle(cloneForParent.get());
> 
> Why is this safe? It looks cloneForParent is gone after this function.
> How about just make State::m_parentStyle RefPtr?

Done.
Comment 7 WebKit Review Bot 2013-03-15 01:26:37 PDT 
Comment on attachment 193251 [details]
Patch

Clearing flags on attachment: 193251

Committed r145885: <http://trac.webkit.org/changeset/145885>
Comment 8 WebKit Review Bot 2013-03-15 01:26:40 PDT 
All reviewed patches have been landed.  Closing bug.