Bug 112322 – Crash at RenderStyle::inheritFrom reported by fuzzer

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Takashi Sakamoto]

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

[Takashi Sakamoto]

https://cluster-fuzz.appspot.com/testcase?key=159273524

[Takashi Sakamoto]

Created attachment 193080 [details]
Patch

[Mike West]

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.

[Hajime Morrita]

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?

[Takashi Sakamoto]

Created attachment 193251 [details]
Patch

[Takashi Sakamoto]

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.

[WebKit Review Bot]

Comment on attachment 193251 [details]
Patch

Clearing flags on attachment: 193251

Committed r145885: <http://trac.webkit.org/changeset/145885>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.