Bug 133825 - AX: Safari crashed once in WebCore::AccessibilityObject::ariaIsHidden
Summary: AX: Safari crashed once in WebCore::AccessibilityObject::ariaIsHidden
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-12 14:51 PDT by chris fleizach
Modified: 2014-06-16 14:44 PDT (History)
12 users (show)

See Also:


Attachments
patch (2.47 KB, patch)
2014-06-12 14:54 PDT, chris fleizach
enrica: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2014-06-12 14:51:22 PDT
Crash log here

#0	0x00000001005deee0 in WebCore::AccessibilityObject::element() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:1874
#1	0x00000001005d9710 in WebCore::AccessibilityObject::getAttribute(WebCore::QualifiedName const&) const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:1686
#2	0x00000001005edd54 in WebCore::AccessibilityObject::isARIAHidden() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:2373
#3	0x00000001005ede80 in WebCore::AccessibilityObject::defaultObjectInclusion() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:2393
#4	0x00000001005f22f8 in WebCore::AccessibilityRenderObject::defaultObjectInclusion() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1166
#5	0x00000001005f2370 in WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1178
#6	0x00000001005edfe8 in WebCore::AccessibilityObject::accessibilityIsIgnored() const at /Code/Web/OpenSource/Source/WebCore/accessibility/AccessibilityObject.cpp:2420
#7	0x00000001005d2bf0 in WebCore::AXObjectCache::notificationPostTimerFired(WebCore::Timer<WebCore::AXObjectCache>&) at /Code/Web/OpenSource/Source/WebCore/accessibility/AXObjectCache.cpp:738
#8	0x0000000100633030 in decltype(*(std::__1::forward<WebCore::AXObjectCache*&>(fp0)).*fp(std::__1::forward<WebCore::Timer<WebCore::AXObjectCache>&>(fp1))) std::__1::__invoke<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, WebCore::Timer<WebCore::AXObjectCache>&, void>(void (WebCore::AXObjectCache::*&&&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&&&, WebCore::Timer<WebCore::AXObjectCache>&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/__functional_base:380
#9	0x0000000100632fc0 in std::__1::__bind_return<void (WebCore::AXObjectCache::*)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, std::__1::tuple<>, _is_valid_bind_return<void (WebCore::AXObjectCache::*)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::AXObjectCache::*)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, 0ul, 1ul, std::__1::tuple<> >(void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >&, std::__1::__tuple_indices<0ul, 1ul>, std::__1::tuple<>&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/functional:2022
#10	0x0000000100632f78 in std::__1::__bind_return<void (WebCore::AXObjectCache::*)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, std::__1::tuple<>, _is_valid_bind_return<void (WebCore::AXObjectCache::*)(WebCore::Timer<WebCore::AXObjectCache>&), std::__1::tuple<WebCore::AXObjectCache*, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >::operator()<>() [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/functional:2085
#11	0x0000000100632f60 in decltype(std::__1::forward<std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >&>(std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/__functional_base:413
#12	0x0000000100632f60 in std::__1::__function::__func<std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > >, std::__1::allocator<std::__1::__bind<void (WebCore::AXObjectCache::*&)(WebCore::Timer<WebCore::AXObjectCache>&), WebCore::AXObjectCache*&, std::__1::reference_wrapper<WebCore::Timer<WebCore::AXObjectCache> > > >, void ()>::operator()() at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/functional:1370
#13	0x0000000100632230 in std::__1::function<void ()>::operator()() const at /Applications/Xcode.app/Contents/Developer/Toolchains/iOS8.0.xctoolchain/usr/bin/../include/c++/v1/functional:1755
#14	0x00000001006321ec in WebCore::Timer<WebCore::AXObjectCache>::fired() at /Code/Web/OpenSource/Source/WebCore/platform/Timer.h:133
#15	0x00000001022a1a38 in WebCore::ThreadTimers::sharedTimerFiredInternal() at /Code/Web/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:132
#16	0x00000001022a1730 in WebCore::ThreadTimers::sharedTimerFired() at /Code/Web/OpenSource/Source/WebCore/platform/ThreadTimers.cpp:107
#17	0x0000000101fe8004 in WebCore::timerFired(__CFRunLoopTimer*, void*) at /Code/Web/OpenSource/Source/WebCore/platform/ios/SharedTimerIOS.mm:62
Comment 1 chris fleizach 2014-06-12 14:51:42 PDT
The problem looks like if you ask axIsIgnored() on a newly created object, it can actually deallocate itself
Comment 2 Radar WebKit Bug Importer 2014-06-12 14:52:05 PDT
<rdar://problem/17292966>
Comment 3 chris fleizach 2014-06-12 14:54:38 PDT
Created attachment 232992 [details]
patch
Comment 4 Geoffrey Garen 2014-06-16 13:15:20 PDT
Comment on attachment 232992 [details]
patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:433
> +    // Sometimes asking accessibilityIsIgnored() will cause the newObject to be deallocated, and then
> +    // it will disappear when this function is finished, leading to a use-after-free.
> +    if (newObj->isDetached())
> +        return nullptr;

Do you really mean deallocated, or just detached? If newObj is truly deallocated, we need to understand why our RefPtr didn't prevent that. Accessing newObj after it has been deallocated is not sound, and isDetached() might return anything if it's called on a deallocated object.
Comment 5 chris fleizach 2014-06-16 13:22:29 PDT
(In reply to comment #4)
> (From update of attachment 232992 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232992&action=review
> 
> > Source/WebCore/accessibility/AXObjectCache.cpp:433
> > +    // Sometimes asking accessibilityIsIgnored() will cause the newObject to be deallocated, and then
> > +    // it will disappear when this function is finished, leading to a use-after-free.
> > +    if (newObj->isDetached())
> > +        return nullptr;
> 
> Do you really mean deallocated, or just detached? If newObj is truly deallocated, we need to understand why our RefPtr didn't prevent that. Accessing newObj after it has been deallocated is not sound, and isDetached() might return anything if it's called on a deallocated object.

I mean detached. The RefPtr<> in this method is still holding onto the object while we're in the context of the methods stack. as soon as we leave that stack and the RefPtr goes away, the object then becomes deallocated
Comment 6 Enrica Casucci 2014-06-16 13:32:24 PDT
Comment on attachment 232992 [details]
patch

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

> Source/WebCore/ChangeLog:8
> +        Sometimes asking accessibilityIsIgnored() will cause a newObject to be deallocated. It will

Please fix the ChangeLog to use detached.
Comment 7 chris fleizach 2014-06-16 14:44:51 PDT
http://trac.webkit.org/changeset/170028