Bug 241588 - General Protection Fault in WebKitWebProcess on 32bit CPUs
Summary: General Protection Fault in WebKitWebProcess on 32bit CPUs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P2 Major
Assignee: Nobody
URL: https://www.microsoft.com/en-us/softw...
Keywords: DoNotImportToRadar
Depends on:
Blocks:
 
Reported: 2022-06-13 23:09 PDT by karogyoker2+webkit
Modified: 2022-06-17 17:16 PDT (History)
14 users (show)

See Also:


Attachments
backtrace and other (45.76 KB, text/plain)
2022-06-13 23:09 PDT, karogyoker2+webkit
no flags Details
git diff (1.78 KB, patch)
2022-06-16 17:17 PDT, karogyoker2+webkit
ysuzuki: review+
ysuzuki: commit-queue-
Details | Formatted Diff | Diff
patch (1.79 KB, patch)
2022-06-17 14:41 PDT, karogyoker2+webkit
ysuzuki: review+
Details | Formatted Diff | Diff
Patch (1.78 KB, patch)
2022-06-17 15:18 PDT, karogyoker2+webkit
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description karogyoker2+webkit 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
Comment 1 karogyoker2+webkit 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/
Comment 2 karogyoker2+webkit 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
Comment 3 karogyoker2+webkit 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?
Comment 4 Yusuke Suzuki 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)
Comment 5 karogyoker2+webkit 2022-06-17 14:41:32 PDT
Created attachment 460311 [details]
patch

Thanks for reviewing.
Changes applied requested by review.
Comment 6 Yusuke Suzuki 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.
Comment 7 karogyoker2+webkit 2022-06-17 15:18:05 PDT
Created attachment 460313 [details]
Patch
Comment 8 EWS 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.