Bug 241588

Summary: General Protection Fault in WebKitWebProcess on 32bit CPUs
Product: WebKit Reporter: karogyoker2+webkit
Component: Web AudioAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: berto, cdumez, eric.carlson, ews-watchlist, glenn, jer.noble, mikhail, philipj, pmatos, sergio, xan.lopez, youssefdevelops, y_soliman, ysuzuki
Priority: P2 Keywords: DoNotImportToRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
URL: https://www.microsoft.com/en-us/software-download/windows10ISO
Attachments:
Description Flags
backtrace and other
none
git diff
ysuzuki: review+, ysuzuki: commit-queue-
patch
ysuzuki: review+
Patch none

karogyoker2+webkit
Reported 2022-06-13 23:09:41 PDT
Created attachment 460224 [details] backtrace and other Dear Maintainer, The problem is reproducible if I try to open this webpage in epiphany-browser: https://www.microsoft.com/en-us/software-download/windows10ISO The problem is that Gnome Web is displaying Oops! Something went wrong while displaying this page. Please reload or visit a different page to continue. Instead it should display the page I want to open. I'm using the latest Debian Testing i386. Package: libwebkit2gtk-4.1-0 Version: 2.36.3-1 cat /var/log/kern.log | grep webkit debian kernel: [ 6021.658455] traps: ffline renderer[26566] general protection fault ip:b5717218 sp:983a6f50 error:0 in libwebkit2gtk-4.1.so.0.1.7[b440b000+27af000 I've attached the gdb output and system information. Based on the stack trace, this is where the segfault happens: https://github.com/WebKit/WebKit/blob/623a598f89fff02777796a87d35942a8dfe5a621/Source/WebCore/platform/audio/DenormalDisabler.h#L59 In the gdb logs it can be seen that mxcsr has the default value 0x1f80. Bit-wise OR-ing this with 0x8040 gives 0x9fc0 as it is seen in eax. That is 1001 1111 1100 0000 in binary. I'm using zero-based indexing from now on. The 6th bit is 1, and this is a problem because setting the 6th bit is reserved and setting it to 1 is not allowed and gives a general protection exception[1]. Flag meanings here[2]. It doesn't cause a GP exception on newer hardware because the 6th bit is not reserved anymore. I've written a little C++ program to test this (at the end of the attached file). Trying to set the 6th bit to 1 on an Athlon XP crashes the program but on a Haswell it is allowed. Therefore it is not reproducible on QEMU, only on real 32 bit hardware[3]. Why an ISO download page is using Web Audio API? Most probably because of browser fingerprinting purposes. Since a lot of websites do that, all those pages will crash and WebKit is unable to display them on Debian i386. This is a baseline violation on Debian i386, hence the severity is major. Workarounds: - Turn off "Website Data Storage" in Preferences (in GNOME Web). - Disable Web Audio API somehow Possible fix: Delete "defined(__i386__) || " from line #39 and #86 in DenormalDisabler.h#L59. 0x8040 is also used here, so this should be fixed as well (not related to current crash): https://github.com/WebKit/WebKit/blob/623a598f89fff02777796a87d35942a8dfe5a621/Source/ThirdParty/libwebrtc/Source/webrtc/system_wrappers/source/denormal_disabler.cc#L18 Change WEBRTC_ARCH_X86_FAMILY to WEBRTC_ARCH_X86_64. I was not able to test these fixes because I got an error at step Tools/Scripts/update-webkitgtk-libs: Installing from webkit-sdk org.webkit.Platform i686 21.08 Looking for matches... error: Nothing matches org.webkit.Platform in remote webkit-sdk The following command returned a non-zero exit status: flatpak install --user --assumeyes webkit-sdk org.webkit.Platform --reinstall 21.08 Output: None Died at Tools/Scripts/update-webkitgtk-libs line 28. [1]: https://qcd.phys.cmu.edu/QCDcluster/intel/vtune/reference/vc148.htm [2]: https://help.totalview.io/previous_releases/2019/html/Reference_Guide/Intelx86MXSCRRegister_2.html [3]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1012548#10
Attachments
backtrace and other (45.76 KB, text/plain)
2022-06-13 23:09 PDT, karogyoker2+webkit
no flags
git diff (1.78 KB, patch)
2022-06-16 17:17 PDT, karogyoker2+webkit
ysuzuki: review+
ysuzuki: commit-queue-
patch (1.79 KB, patch)
2022-06-17 14:41 PDT, karogyoker2+webkit
ysuzuki: review+
Patch (1.78 KB, patch)
2022-06-17 15:18 PDT, karogyoker2+webkit
no flags
karogyoker2+webkit
Comment 1 2022-06-14 10:15:22 PDT
A little background[1] on the topic. "Executing a program on a Pentium III processor enables the FTZ flag, but not DAZ."[2] DAZ is not supported by Pentium 3[3] nor VIA Nehemiah[3]: (This is VIA Nehemiah[4].) DAZ is also not supported by AMD Athlon XP. (I have one and I can see.) "Some processor steppings support SSE2 but do not support the DAZ mode."[5] From page #60[5] there is an example how to detect DAZ support. "Initial steppings of PentiumĀ® 4 processors did not support DAZ."[6] An incorrect way to detect DAZ[7]. As we can see above[6], there is no guarantee that if SSE2 is available then DAZ is supported as well. "But is this for all i386 CPUs or only for older models? How come this never crashed before?"[8] Because browser-fingerprinter scripts which are utilizing Web Audio API were not that widespread as they are now. Also, not many people are using real 32 bit CPUs. The proper solution would be to detect DAZ support. We can keep using the 0x8000 mask if there is SSE as it is now. But if there is DAZ as well, use 0x8040. This way we can get the most optimal performance on most CPUs. If SSE2 detection happens all around, this should be detected as well. The number of CPU steppings having DAZ support is even less than those having SSE2 support. As the usage of Web Audio API for browser-fingerprinting[9] gets even more ubiquitous, this segfault will happen more and more often. [1]: https://en.wikipedia.org/wiki/Subnormal_number [2]: http://physics.ujep.cz/~zmoravec/prga/main_for/mergedProjects/optaps_for/common/optaps_dsp_rtme.htm [3]: https://www.carlh.net/plugins/denormals.php [4]: https://en.wikipedia.org/wiki/List_of_VIA_Eden_microprocessors#%22Nehemiah%22_(130_nm) [5]: https://bochs.sourceforge.io/techspec/24161821.pdf [6]: http://web.archive.org/web/20111101165633/http://software.intel.com/en-us/articles/x87-and-sse-floating-point-assists-in-ia-32-flush-to-zero-ftz-and-denormals-are-zero-daz/ [7]: https://bugs.webkit.org/show_bug.cgi?id=134060 [8]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1012548#47 [9]: https://fingerprint.com/blog/audio-fingerprinting/
karogyoker2+webkit
Comment 2 2022-06-15 06:59:13 PDT
> The proper solution would be to detect DAZ support. We can keep using the > 0x8000 mask if there is SSE as it is now. But if there is DAZ as well, use > 0x8040. This way we can get the most optimal performance on most CPUs. Detecting DAZ can be done in run time: #include <cstring> #include <cinttypes> // Precondition: SSE support, but we take that granted for i386 bool isDazSupported() { #if defined(__i386__) #if defined(__x86_64__) return true; #else struct fxsave_area_struct { uint8_t before[28]; uint32_t mxcsr_mask; uint8_t after[480]; } __attribute__ ((aligned (16))); fxsave_area_struct regdata; memset(&regdata, 0, sizeof(fxsave_area_struct)); asm volatile ("fxsave %0" : "=m" (regdata)); return regdata.mxcsr_mask & 0x0040; #endif #else return false; #endif } I don't know where should I put isDazSupported(). This function would be needed for WebRTC as well, because in its code the false assumption was made there as well that all x86 CPU supports DAZ. Also, isDazSupported() should be called only once. In the past there was a run time check for SSE2 support, I can't find it now. I also don't know how the 32 bit WebKit build without SSE2 is being built currently on 64 bit systems. The DAZ support might be handled similarly. We could have a simpler solution: Treat all 64 bit builds DAZ compatible and all 32 bit builds DAZ incompatible. In this case the drawback would be that 32 bit builds would have a performance degradation on all 64 bit CPUs and on some newer 32 bit Pentium 4 CPUs. In this case the fix in DenormalDisabler.h would be pretty straightforward. #if CPU(X86_64) setCSR(m_savedCSR | 0x8040); #else setCSR(m_savedCSR | 0x8000); #endif Then a similar fix could be done in denormal_disabler.cc. // Control register bit mask to disable denormals on the hardware. #if defined(WEBRTC_DENORMAL_DISABLER_X86_SUPPORTED) #if defined(WEBRTC_ARCH_X86_64) // On x86_64 two bits are used: flush-to-zero (FTZ) and denormals-are-zero (DAZ). constexpr int kDenormalBitMask = 0x8040; #else // On x86 one bit is used: flush-to-zero (FTZ). constexpr int kDenormalBitMask = 0x8000; #endif #elif defined(WEBRTC_ARCH_ARM_FAMILY) // On ARM one bit is used: flush-to-zero (FTZ). constexpr int kDenormalBitMask = 1 << 24; #endif
karogyoker2+webkit
Comment 3 2022-06-16 17:17:33 PDT
Created attachment 460286 [details] git diff I have coded the fix. I built it on a 64 bit Debian install, seems to work fine. But I'd like to test it on my physical 32 bit CPU before I open a PR on GitHub. I want to build it on the same Debian install where I built the 64 bit version. The problem is that I couldn't figure out yet how to build a 32 bit binary. I tried build-webkit --gtk --32-bit --makeargs="-j3" but libwebkit2gtk-4.1.so.0.2.0 is still a 64 bit binary. "ELF 64-bit LSB shared object, x86-64", that's what `file` says. Should I clear the output folders somehow and rebuild all? Or something completely different is needed?
Yusuke Suzuki
Comment 4 2022-06-17 13:47:29 PDT
Comment on attachment 460286 [details] git diff View in context: https://bugs.webkit.org/attachment.cgi?id=460286&action=review > Source/WebCore/platform/audio/DenormalDisabler.h:40 > +#if defined(__GNUC__) && defined(__SSE__) Use #if COMPILER(GCC_COMPATIBLE) && defined(__SSE__) > Source/WebCore/platform/audio/DenormalDisabler.h:87 > +#if defined(__GNUC__) && defined(__SSE__) Use #if COMPILER(GCC_COMPATIBLE) && defined(__SSE__) > Source/WebCore/platform/audio/DenormalDisabler.h:90 > +#if defined(__x86_64__) Use #if CPU(X86_64)
karogyoker2+webkit
Comment 5 2022-06-17 14:41:32 PDT
Created attachment 460311 [details] patch Thanks for reviewing. Changes applied requested by review.
Yusuke Suzuki
Comment 6 2022-06-17 14:46:23 PDT
(In reply to karogyoker2+webkit from comment #5) > Created attachment 460311 [details] > patch > > Thanks for reviewing. > Changes applied requested by review. Oops, you need to use webkit-patch command to upload a patch. It lacks COMMIT_MESSAGE change so we cannot land it.
karogyoker2+webkit
Comment 7 2022-06-17 15:18:05 PDT
EWS
Comment 8 2022-06-17 17:16:36 PDT
Committed r295652 (251657@main): <https://commits.webkit.org/251657@main> Reviewed commits have been landed. Closing PR #1597 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.