Bug 167631 – [CoordinatedGraphics] WebCoordinatedSurface::create should do null-check of the return value of ShareableBitmap::createShareable

• Component: WebKit2
• Status: RESOLVED FIXED

[Fujii Hironori]

WebCoordinatedSurface::create does not do null-check of the return value of ShareableBitmap::createShareable.

WebCoordinatedSurface.cpp is used only in EFL port at the moment.

This causes a crash with following call stack in case of out of shared memory:

> Thread 1 "WebKitWebProces" received signal SIGSEGV, Segmentation fault.
> 0x00007f5dca7311c0 in WebKit::ShareableBitmap::data() const () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> (gdb) bt
> #0  0x00007f5dca7311c0 in WebKit::ShareableBitmap::data() const () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #1  0x00007f5dca991312 in WebKit::ShareableBitmap::createCairoSurface() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #2  0x00007f5dca991393 in WebKit::ShareableBitmap::createGraphicsContext() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #3  0x00007f5dca98b3e8 in WebKit::WebCoordinatedSurface::createGraphicsContext(WebCore::IntRect const&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #4  0x00007f5dca98b469 in WebKit::WebCoordinatedSurface::paintToSurface(WebCore::IntRect const&, WebCore::CoordinatedSurface::Client&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #5  0x00007f5dca9eef98 in WebKit::UpdateAtlas::paintOnAvailableBuffer(WebCore::IntSize const&, unsigned int&, WebCore::IntPoint&, WebCore::CoordinatedSurface::Client&) () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #6  0x00007f5dcb835ff3 in WebCore::Tile::updateBackBuffer() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #7  0x00007f5dcb4e2319 in WebCore::TiledBackingStore::updateTileBuffers() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #8  0x00007f5dcb4e3fad in WebCore::TiledBackingStore::createTiles(WebCore::IntRect const&, WebCore::IntRect const&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #9  0x00007f5dcb4e41ff in WebCore::TiledBackingStore::createTilesIfNeeded(WebCore::IntRect const&, WebCore::IntRect const&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #10 0x00007f5dcb4e0966 in WebCore::CoordinatedGraphicsLayer::updateContentBuffers() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #11 0x00007f5dcb4e09d3 in WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers() ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #12 0x00007f5dcb4e09fc in WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers() ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #13 0x00007f5dcb4e09fc in WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers() ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #14 0x00007f5dcb4e09fc in WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers() ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #15 0x00007f5dca9ea278 in WebKit::CompositingCoordinator::flushPendingLayerChanges() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #16 0x00007f5dca9e3764 in WebKit::AcceleratedDrawingArea::sendDidUpdateBackingStoreState() ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #17 0x00007f5dca9e3e3c in WebKit::AcceleratedDrawingArea::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #18 0x00007f5dcaa3b15e in WebKit::DrawingArea::didReceiveMessage(IPC::Connection&, IPC::Decoder&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #19 0x00007f5dca7202e9 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #20 0x00007f5dca866e76 in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #21 0x00007f5dca71da8b in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #22 0x00007f5dca71e848 in IPC::Connection::dispatchOneMessage() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #23 0x00007f5dcb94d6d1 in WTF::RunLoop::performWork() () from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #24 0x00007f5dc852eb2e in _ecore_pipe_handler_call (p=p@entry=0x1f26ac0, buf=0x2217be0 "W^\"\002", len=<optimized out>)
>     at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_pipe.c:511
> #25 0x00007f5dc852f1e9 in _ecore_pipe_read (data=0x1f26ac0, fd_handler=<optimized out>)
>     at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_pipe.c:637
> #26 0x00007f5dc852cb82 in _ecore_call_fd_cb (fd_handler=0x1f1cab0, data=<optimized out>, func=<optimized out>)
>     at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_private.h:333
> #27 _ecore_main_fd_handlers_call () at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_main.c:1974
> #28 _ecore_main_loop_iterate_internal (once_only=once_only@entry=0)
>     at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_main.c:2339
> #29 0x00007f5dc852cf67 in ecore_main_loop_begin () at /home/fujii/work/webkit/ga/WebKitBuild/DependenciesEFL/Source/efl-1.18.4/src/lib/ecore/ecore_main.c:1286
> #30 0x00007f5dca9f08cd in int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**) ()
>    from /home/fujii/work/webkit/ga/WebKitBuild/Release/lib/libewebkit2.so.1
> #31 0x00007f5dc931b830 in __libc_start_main (main=0x400760 <main>, argc=2, argv=0x7ffdd6abb108, init=<optimized out>, fini=<optimized out>, 
>     rtld_fini=<optimized out>, stack_end=0x7ffdd6abb0f8) at ../csu/libc-start.c:291
> #32 0x00000000004007b9 in _start ()

[Fujii Hironori]

Created attachment 300191 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 300191 [details]
Patch

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

> Source/WebKit2/Shared/CoordinatedGraphics/WebCoordinatedSurface.cpp:84
> +    RefPtr<ShareableBitmap> bitmap = ShareableBitmap::createShareable(size, (flags & SupportsAlpha) ? ShareableBitmap::SupportsAlpha : ShareableBitmap::NoFlags);
> +    if (!bitmap)
> +        return nullptr;
> +    surface = create(size, flags, WTFMove(bitmap));

So, here we don't need the surface anymore. I think this method could be simplified with something like:

#if USE(GRAPHICS_SURFACE)
if (auto surface = createWithSurface(size, flags))
    return surface;
#endif

if (auto bitmap = ShareableBitmap::createShareable(size, (flags & SupportsAlpha) ? ShareableBitmap::SupportsAlpha : ShareableBitmap::NoFlags))
    return create(size, flags, WTFMove(bitmap));

return nullptr;

[Fujii Hironori]

Created attachment 300196 [details]
Patch

Thank you for reviewing my patch. Revised the patch.

[WebKit Commit Bot]

Comment on attachment 300196 [details]
Patch

Clearing flags on attachment: 300196

Committed r211414: <http://trac.webkit.org/changeset/211414>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.