[GTK] Layout Test webrtc/video.html issues "stack smashing detected" webrtc/video.html issues "stack smashing detected" in my linux box (Ubuntu 17.04) while build bot does't. I'm using Gtk port, trunk@218806, Release build. > fujii@ubuntu $ ./Tools/Scripts/run-webkit-tests --gtk --release --no-new-test-results -v --iterations=3 webrtc/video.html > Using port 'gtk-wk2' > Test configuration: <, x86, release> > Placing test results in /home/fujii/work/webkit/ga/WebKitBuild/Release/layout-test-results > Baseline search path: platform/gtk -> platform/wk2 -> generic > Using Release build > Pixel tests disabled > Regular timeout: 15000, slow test timeout: 75000 > Command line: /home/fujii/work/webkit/ga/Tools/jhbuild/jhbuild-wrapper --gtk run /home/fujii/work/webkit/ga/WebKitBuild/Release/bin/WebKitTestRunner - > > Found 1 test; running 1 (3 times each: --repeat-each=1 --iterations=3), skipping 0. > > Running 1 test > > Running 1 WebKitTestRunner. > > [1/3] webrtc/video.html*** stack smashing detected ***: /home/fujii/work/webkit/ga/WebKitBuild/Release/bin/WebKitWebProcess terminated > [1/3] webrtc/video.html failed (text diff) > [2/3] webrtc/video.html*** stack smashing detected ***: /home/fujii/work/webkit/ga/WebKitBuild/Release/bin/WebKitWebProcess terminated > [2/3] webrtc/video.html failed (text diff) > [3/3] webrtc/video.html*** stack smashing detected ***: /home/fujii/work/webkit/ga/WebKitBuild/Release/bin/WebKitWebProcess terminated > [3/3] webrtc/video.html failed (text diff) > > All 3 tests ran as expected.
Created attachment 313896 [details] WIP patch It seems that g_object_get needs to be passed an int pointer instread of a bool pointer. WIP patch solved the stack smashing. But, there are other similar codes using g_object_get and bool pointers. > bool MediaPlayerPrivateGStreamerBase::muted() const > { > if (!m_volumeElement) > return false; > > bool muted; > g_object_get(m_volumeElement.get(), "mute", &muted, nullptr); > return muted; > } Does anyone have any thought?
Comment on attachment 313896 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=313896&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerOwr.cpp:364 > + int audioDisabled; > + int videoDisabled; Use gboolean instead of int.
(In reply to Fujii Hironori from comment #1) > Created attachment 313896 [details] > WIP patch > > It seems that g_object_get needs to be passed an int pointer instread of a > bool pointer. Yes, this has caused problems in the past. > WIP patch solved the stack smashing. > > But, there are other similar codes using g_object_get and bool pointers. > > > bool MediaPlayerPrivateGStreamerBase::muted() const > > { > > if (!m_volumeElement) > > return false; > > > > bool muted; > > g_object_get(m_volumeElement.get(), "mute", &muted, nullptr); > > return muted; > > } > > Does anyone have any thought? We should fix all of them, by using gboolean instead of bool.
You're right. I made this mistake once before, and it took hours before I realized the problem. Of course a GObject property is going to be a gboolean (an int), not a C++ bool (one byte). This is such an easy mistake to make when not being very careful. If you pass just one byte, then g_object_get() will write three bytes out of bounds. It's probably impractical to audit the entire codebase for this issue, but we should check the revision that introduced the bug to check for other misuses, and also check surrounding code.
Created attachment 313900 [details] Patch Thank you, KaL and Michael. I did audit other usages. I didn't find any other problem.
Comment on attachment 313900 [details] Patch Excellent, thank you!
Comment on attachment 313900 [details] Patch Clearing flags on attachment: 313900 Committed r218832: <http://trac.webkit.org/changeset/218832>
All reviewed patches have been landed. Closing bug.