New issue
Advanced search Search tips

Issue 746914 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Incorrect-function-pointer-type in gl::EGLApiBase::eglGetDisplayFn

Project Member Reported by ClusterFuzz, Jul 20 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6467460651024384

Fuzzer: libFuzzer_gpu_swiftshader_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Incorrect-function-pointer-type
Crash Address: 
Crash State:
  gl::EGLApiBase::eglGetDisplayFn
  gl::GLSurfaceEGL::InitializeDisplay
  gl::GLSurfaceEGL::InitializeOneOff
  
Sanitizer: undefined (UBSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=487975:488087

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6467460651024384


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Jul 20 2017

Labels: M-61
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 20 2017

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 3 by sheriffbot@chromium.org, Jul 20 2017

Labels: Pri-1
Components: UI>GFX
Owner: geoffl...@chromium.org
Status: Assigned (was: Untriaged)
It looks like an incorrect function pointer type in auto generated bindings? Can you take a look geofflang@?
Owner: capn@chromium.org
Assigning to Nicolas because this is the swiftshader fuzzer.  This sounds like an issue they've dealt with before.

Comment 6 by capn@chromium.org, Jul 21 2017

Cc: geoffl...@chromium.org p...@chromium.org cwallez@chromium.org
Components: -UI>GFX Internals>GPU>SwiftShader
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam -Security_Severity-Medium -Security_Impact-Head -ReleaseBlock-Stable -Stability-UndefinedBehaviorSanitizer ReleaseBlock-NA Type-Bug
Status: Started (was: Assigned)
Just like with  Issue 737384 , the function pointer type is actually correct. This is another false positive, but this time on the Chrome side instead of within SwiftShader (although the pointer is retrieved from SwiftShader).

Interestingly when I remove GPU_FUZZER_USE_SWIFTSHADER from gpu/BUILD.gn, so that ANGLE is being used instead, the test doesn't fail. So something is different about how ANGLE is getting built/linked that UBSan likes better.

@pcc, do you have any clues what might be going on this time? I've tried disabling our DSO version script entirely to give all symbols default visibility, but that didn't help. Thanks.

Comment 7 by p...@chromium.org, Jul 21 2017

I think that what is happening here is that the function eglGetDisplay takes a value of type EGLNativeDisplayType, which is typedef'd to the _XDisplay struct defined in Xlib/X11.h, and because chrome and libEGL.so are compiled with -fvisibility=hidden, the definition of _XDisplay receives hidden visibility, and therefore so does any function typeinfo that mentions that type.

It isn't clear to me what the correct fix is here (I believe that the visibility rules for typeinfo are dictated by the C++ ABI, so we may need to consider switching from typeinfo to something with more useful visibility rules), but probably the best workaround I can think of for now would be to add the attribute __attribute__((no_sanitize("function"))) to the definition of eglGetDisplay or any other libEGL API functions that take or return an _XDisplay.

It looks like the problem doesn't affect ANGLE because the ANGLE codepath uses eglGetPlatformDisplayEXT instead of eglGetDisplay, which passes the reference to with a parameter of type void* instead of _XDisplay*: https://cs.chromium.org/chromium/src/ui/gl/gl_surface_egl.cc?type=cs&l=222

I've included a list of potentially affected function types below. If any of them are used as part of libEGL's public API, we will most likely need to use the attribute on those functions as well.

$ nm libfuzzer-linux-release-488087/swiftshader/libEGL.so | grep 'd _ZTIF' | c++filt
0000000000027110 d typeinfo for bool (egl::Context*)
0000000000027b20 d typeinfo for bool (egl::Display*)
0000000000027b50 d typeinfo for bool (egl::Display*, egl::Context*)
0000000000027b40 d typeinfo for bool (egl::Display*, egl::Surface*)
0000000000027b30 d typeinfo for bool (egl::Display*, void*)
0000000000027120 d typeinfo for bool (egl::Surface*)
0000000000027150 d typeinfo for bool (egl::FenceSync*)
0000000000026fb8 d typeinfo for bool (egl::Config const**, egl::Config const**, egl::SortConfig&)
00000000000271f8 d typeinfo for int (Screen*)
0000000000027030 d typeinfo for int (_XDisplay*)
0000000000027140 d typeinfo for int (_XDisplay*, unsigned long, XWindowAttributes*)
00000000000273c8 d typeinfo for unsigned int (egl::Image*)
0000000000026f88 d typeinfo for unsigned int (egl::Config const**, egl::Config const**, egl::Config const**, egl::SortConfig&)
0000000000026f98 d typeinfo for unsigned int (egl::Config const**, egl::Config const**, egl::Config const**, egl::Config const**, egl::SortConfig&)
0000000000026fa8 d typeinfo for unsigned int (egl::Config const**, egl::Config const**, egl::Config const**, egl::Config const**, egl::Config const**, egl::SortConfig&)
0000000000027368 d typeinfo for unsigned long (egl::Context* const&)
0000000000027338 d typeinfo for unsigned long (egl::Surface* const&)
0000000000027398 d typeinfo for unsigned long (egl::FenceSync* const&)
0000000000027418 d typeinfo for std::__1::__tree_iterator<std::__1::__value_type<unsigned int, egl::Image*>, std::__1::__tree_node<std::__1::__value_type<unsigned int, egl::Image*>, void*>*, long> (std::__1::__tree_const_iterator<std::__1::__value_type<unsigned int, egl::Image*>, std::__1::__tree_node<std::__1::__value_type<unsigned int, egl::Image*>, void*>*, long>)
0000000000027388 d typeinfo for std::__1::__tree_iterator<egl::Context*, std::__1::__tree_node<egl::Context*, void*>*, long> (std::__1::__tree_const_iterator<egl::Context*, std::__1::__tree_node<egl::Context*, void*>*, long>)
0000000000027378 d typeinfo for std::__1::__tree_iterator<egl::Context*, std::__1::__tree_node<egl::Context*, void*>*, long> (egl::Context* const&)
0000000000027358 d typeinfo for std::__1::__tree_iterator<egl::Surface*, std::__1::__tree_node<egl::Surface*, void*>*, long> (std::__1::__tree_const_iterator<egl::Surface*, std::__1::__tree_node<egl::Surface*, void*>*, long>)
0000000000027348 d typeinfo for std::__1::__tree_iterator<egl::Surface*, std::__1::__tree_node<egl::Surface*, void*>*, long> (egl::Surface* const&)
00000000000273b8 d typeinfo for std::__1::__tree_iterator<egl::FenceSync*, std::__1::__tree_node<egl::FenceSync*, void*>*, long> (std::__1::__tree_const_iterator<egl::FenceSync*, std::__1::__tree_node<egl::FenceSync*, void*>*, long>)
00000000000273a8 d typeinfo for std::__1::__tree_iterator<egl::FenceSync*, std::__1::__tree_node<egl::FenceSync*, void*>*, long> (egl::FenceSync* const&)
0000000000026f18 d typeinfo for std::__1::pair<std::__1::__tree_iterator<egl::Config, std::__1::__tree_node<egl::Config, void*>*, long>, bool> (egl::Config const&, egl::Config const&)
00000000000273e8 d typeinfo for std::__1::pair<std::__1::__tree_iterator<std::__1::__value_type<unsigned int, egl::Image*>, std::__1::__tree_node<std::__1::__value_type<unsigned int, egl::Image*>, void*>*, long>, bool> (unsigned int const&, std::__1::pair<unsigned int const, egl::Image*>&&)
0000000000027318 d typeinfo for std::__1::pair<std::__1::__tree_iterator<egl::Context*, std::__1::__tree_node<egl::Context*, void*>*, long>, bool> (egl::Context* const&, egl::Context* const&)
0000000000027308 d typeinfo for std::__1::pair<std::__1::__tree_iterator<egl::Surface*, std::__1::__tree_node<egl::Surface*, void*>*, long>, bool> (egl::Surface* const&, egl::Surface* const&)
0000000000027328 d typeinfo for std::__1::pair<std::__1::__tree_iterator<egl::FenceSync*, std::__1::__tree_node<egl::FenceSync*, void*>*, long>, bool> (egl::FenceSync* const&, egl::FenceSync* const&)
0000000000027ba0 d typeinfo for LibEGLexports* ()
0000000000027bb0 d typeinfo for LibX11exports* ()
00000000000272c8 d typeinfo for LibGLESv2exports* ()
0000000000027288 d typeinfo for LibGLES_CMexports* ()
00000000000271e8 d typeinfo for Screen* (_XDisplay*)
0000000000026fd8 d typeinfo for _XDisplay* (char*)
0000000000026f08 d typeinfo for egl::Config const* (void*)
00000000000277a8 d typeinfo for sw::FrameBuffer* (void*, unsigned long, int, int)
00000000000275c8 d typeinfo for egl::Image* (int, int, sw::Format, int)
0000000000027408 d typeinfo for egl::Image* (unsigned int)
00000000000271c8 d typeinfo for egl::Image* (void*)
00000000000275f8 d typeinfo for egl::Image* ()
00000000000270a0 d typeinfo for egl::Context* (egl::Display*, egl::Context const*, int, egl::Config const*)
0000000000027090 d typeinfo for egl::Context* (egl::Display*, egl::Context const*, egl::Config const*)
0000000000027b80 d typeinfo for egl::Context* ()
0000000000027b70 d typeinfo for egl::Current* ()
0000000000026fc8 d typeinfo for egl::Display* (void*)
0000000000027b90 d typeinfo for egl::Surface* ()
00000000000278d8 d typeinfo for void* (_XDisplay*)
0000000000027170 d typeinfo for void* (egl::Image*)
00000000000270b0 d typeinfo for void* (egl::Context*)
0000000000027080 d typeinfo for void* (void*, egl::Context const*, int)
0000000000026f28 d typeinfo for std::__1::__tree_node_base<void*>*& (std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>*&, egl::Config const&)
0000000000026e78 d typeinfo for void (sw::Format, int, int, sw::Format, sw::Format, int, bool)
0000000000026ed8 d typeinfo for void (sw::Format, int, int, sw::Format, sw::Format, int)
0000000000026f68 d typeinfo for void (egl::Config const*&&)
0000000000027478 d typeinfo for void (egl::Display const*, egl::Config const*)
00000000000270d0 d typeinfo for void (egl::Context*)
00000000000277b8 d typeinfo for void (egl::Display*, egl::Config const*, int, int, unsigned int, unsigned int, unsigned int)
0000000000027648 d typeinfo for void (egl::Display*, egl::Config const*, unsigned long)
00000000000270c0 d typeinfo for void (egl::Surface*)
0000000000027628 d typeinfo for void (egl::Texture*)
00000000000270e0 d typeinfo for void (egl::FenceSync*)
0000000000027268 d typeinfo for void (std::__1::__tree_node<egl::Config, void*>*)
00000000000272f8 d typeinfo for void (std::__1::__tree_node<std::__1::__value_type<unsigned int, egl::Image*>, void*>*)
0000000000027258 d typeinfo for void (std::__1::__tree_node<egl::Context*, void*>*)
0000000000027278 d typeinfo for void (std::__1::__tree_node<egl::Surface*, void*>*)
0000000000027248 d typeinfo for void (std::__1::__tree_node<egl::FenceSync*, void*>*)
0000000000026f38 d typeinfo for void (std::__1::__tree_end_node<std::__1::__tree_node_base<void*>*>*, std::__1::__tree_node_base<void*>*&, std::__1::__tree_node_base<void*>*)
0000000000026f48 d typeinfo for void (std::__1::__tree_node_base<void*>*, std::__1::__tree_node_base<void*>*)
0000000000026f78 d typeinfo for void (egl::Config const**, egl::Config const**, egl::SortConfig&)

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 25 2017

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/506cc5e06043ba0819cc8f7c9190826faf47b1d6

commit 506cc5e06043ba0819cc8f7c9190826faf47b1d6
Author: Nicolas Capens <capn@google.com>
Date: Tue Jul 25 02:20:19 2017

Suppress UBSan false positives.

Function pointers from exported functions are wrongly flagged as being
of incorrect type. This happens both on libEGL and libGLESv2 entry
functions, as well as functions called between them.

 Bug chromium:746914 

Change-Id: I2bf5a8f06546c233ede7a4820c0cda3e997f096e
Reviewed-on: https://swiftshader-review.googlesource.com/10868
Reviewed-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>

[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/CMakeLists.txt
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Android.mk
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Common/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Main/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Main/FrameBufferX11.cpp
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/common/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/compiler/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/compiler/preprocessor/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libEGL/Android.mk
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libEGL/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libEGL/libEGL.vcxproj
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libEGL/main.cpp
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLES_CM/libGLES_CM.vcxproj
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/Android.mk
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/Context.cpp
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/Texture.cpp
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/libGLESv2.cpp
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/OpenGL/libGLESv2/libGLESv2.vcxproj
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Reactor/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Renderer/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/Shader/BUILD.gn
[modify] https://crrev.com/506cc5e06043ba0819cc8f7c9190826faf47b1d6/src/SwiftShader/SwiftShader.vcxproj

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e74c254a7b2e33377d52f6b8bef2ea79463c1edb

commit e74c254a7b2e33377d52f6b8bef2ea79463c1edb
Author: Nicolas Capens <capn@google.com>
Date: Thu Jul 27 04:46:40 2017

Roll SwiftShader 426cb5e..4d3efed

https://swiftshader.googlesource.com/SwiftShader.git/+log/426cb5e..4d3efed

BUG= 746914 

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel,linux_chromium_cfi_rel_ng;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Change-Id: I20a7adff4c99935df35c5c0218002a607795c57f
Reviewed-on: https://chromium-review.googlesource.com/587108
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Commit-Queue: Nicolas Capens <nicolascapens@google.com>
Cr-Commit-Position: refs/heads/master@{#489859}
[modify] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/DEPS

Project Member

Comment 10 by ClusterFuzz, Jul 28 2017

ClusterFuzz has detected this issue as fixed in range 489841:489889.

Detailed report: https://clusterfuzz.com/testcase?key=6467460651024384

Fuzzer: libFuzzer_gpu_swiftshader_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Incorrect-function-pointer-type
Crash Address: 
Crash State:
  gl::EGLApiBase::eglGetDisplayFn
  gl::GLSurfaceEGL::InitializeDisplay
  gl::GLSurfaceEGL::InitializeOneOff
  
Sanitizer: undefined (UBSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=487975:488087
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=489841:489889

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6467460651024384


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Jul 28 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6467460651024384 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-61; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-61 label, otherwise remove Merge-TBD label. Thanks.

Comment 13 by capn@chromium.org, Jul 28 2017

Labels: -Merge-TBD
I don't think a merge is required, since this was a false positive and the suppression only affects UBSan builds.

Sign in to add a comment