QTBUG-69204 - Upstream Qt stylesheet bug causes segmentation faults

[sssoleileraaa]

Description

updateObjects() in qtbase/src/widgets/styles/qstylesheetstyle.cpp can cause a segmentation fault

See https://bugreports.qt.io/browse/QTBUG-69204

For more info on how we reproduced this bug on the client and our temporary fix, see:

• Latest version of PyQt (5.12) is causing segmentation fault #248
• segfault when clicking on Source #254

[redshiftzero]

woops let's leave this one open while we track upstream

[sssoleileraaa]

Debian Buster comes with pyqt 5.11.3 which does not have the fix (fix first available in
5.12.4), so we should wait in case this issue comes back to bite us (our temporary fix no longer appears to no longer be necessary so we're no longer seeing this issue in the client)

[sssoleileraaa]

I think this was closed by accident. I'm going to reopen and provide notes about this upstream bug and how it affects the client.

[sssoleileraaa]

Description

This upstream qt bug can cause a segfault in the client. The fix was added to pyqt 5.12.4, but since we install the system library for pyqt using apt install python3-pyqt5 we still use version 5.11.3. I think we should install a newer version, one with the fix, in the sd-app template VM, and I think this should be part of our workflow for when we encounter major bugs in qt that severly break the client, especially when the bugs have been fixed in later versions. Then we wouldn't have to maintain workarounds, which can be cumbersome and easy to forget about.

How it's affected us

We became aware of QTBUG-69204 in February 2019 while looking into an issue where the application would segfault with the latest version of pyqt (5.12 at the time) whenever a source was selected from the source list. We were on stretch and the system pyqt library was 5.7.x, so it wasn't a major concern at the time, however, we developed a workaround and started tracking the upstream bug. The fix was released in pyqt 5.12.4 a month after we learned about it but we continued to use the system library for pyqt on stretch. Then, in November 2019, we hit another segfault while upgrading sd-app to buster with pyqt 5.11.3 (also installed using apt install python3-pyqt5). After developing another workaround, we didn't see another segfault until last month's segfault during development of making a SpeechBubble's text selectable. Another workaround was added.

Understanding QTBUG-69204

Here's my understanding so far about why we've been seeing segfaults around setting stylesheets and how it was caused by this upstream bug.

A widget's stylesheet has a repolish method that calls updateObjects on all of widget Z's children and grandchilden (grandchildren includes multiple generations). updateObjects triggers a StyleChange event on all children (not grandchildren, at least not until this change was made: qt/qtbase@9b6179c) of widget Z's children and grandchildren. When a widget responds to the StyleChange event, it may end up deleting and readding its children. If this happens, then the repolish method will try to call updateObjects on a widget that no longer exists.

For instance, the following causes a segfault due to this bug:

w = QMainWindow()
s1 = QSplitter()
s2 = QSplitter()
s1.addWidget(s2)
label = QLabel()
label.setTextFormat(Qt.RichText)
s2.addWidget(label)
s1.setStyleSheet('a { b:c; }')
label.setText('hey')
w.setCentralWidget(s1)
w.show()

Here's what's happening in this code:

1. s1's StyleSheet::repolish method is called
2. s1 gets all its children and grandchildren (s2, label, label's QTextFrame) and iteratively calls updateObjects on s2
3. s2's QSplitter::changeEvent executes and deletes and replaces the label's QTextFrame
4. When s1 iteratively calls updateObjects on its grandchild, the QTextFrame that was deleted, it segfaults

QTBUG-69204 Workarounds

Our workaround for the February 2019 segfault was to no longer set the stylesheet on the QApplication so it would no longer repolish all its widgets and trigger StyleChange events. Here's how to repro:

1. Checkout the code before our workaround

git checkout 4117a8d

2. Update logic.py::authenticated to return True
3. Run the client, log in, select a source, see the application segfault

Here's the workaround:

1. Comment out app.setStyleSheet(load_css('sdclient.css')) in app.py::start_app
2. Run the client, log in, select a source, no longer see it sefault

Here's an alternative workaround:

1. Add self.message.setTextFormat(Qt.PlainText) to SpeechBubble.__init__ so the QLabel no longer has a QTextFrame

For the November 2019 segfault workaround, we changed SecureQLabel from RichText to PlainText so that it no longer had a QTextFrame to delete. And last month's workaround was to no longer set the stylesheet on the ConversationView widget so that it would not trigger a StyleChange event that would cause SpeechBubble.message to delete its QTextFrame, which was added as a result of setting SpeechBubble.message to be selectable.

What's next?

One of the goals of #1082 is to move all the CSS into one place and set up a StyleSheet in the application's start method. This makes it much easier for us to know which widgets have StyleSheets that could call the repolish method and trigger the bug. I still think it's a good idea to install a newer version of pyqt in the sd-app template VM and to develop a workflow around it for reasons mentioned in the description (which we can document when we address #1084). What do others think?

If anyone has time to investigate further, here is another way you can reproduce this upstream bug:

app = QApplication(qt_args)
w = QMainWindow()
conversation = QWidget()
conversation_layout = QVBoxLayout()
conversation.setLayout(conversation_layout)
scroll_widget = QWidget()
scroll_widget_layout = QVBoxLayout()
scroll_widget.setLayout(scroll_widget_layout)
scroll = QScrollArea()
scroll.setWidget(scroll_widget)
conversation_layout.addWidget(scroll)
message = QLabel('hey')
message.setTextInteractionFlags(Qt.TextSelectableByMouse)
conversation.setStyleSheet('a { b:c; }')
scroll_widget_layout.addWidget(message)
w.setCentralWidget(conversation)
w.show()
sys.exit(app.exec_())

[kushaldas]

We can not just directly upgrade PyQt as the original issues are in Qt. We will have to upgrade underlying Qt library too. I tried to figure how difficult it would be to maintain our own Qt stack. Every person I asked advised me not to do that, as it takes a dedicated team to maintain it properly. There is a chance that we will break any other random application in the VM which is dependent on Qt

Other options would be somehow use things like pyinstaller and provide our application via that (no clue about security and using along with other Qubes tools). Here is the official page from Qt https://doc.qt.io/qtforpython/deployment.html. Note: people on IRC are not happy how the pyinstaller development stopped. fbs can not do Python 3.7 yet.

Or using a container technology to provide the application in a container (too complex in my mind).

Or moving to Fedora as the template to have updated libraries.

Update:

After having a chat with @redshiftzero I suddenly realized that my knowledge on how PyQt5 gets distributed is old. We can just start using the upstream wheel (importing it to our mirror).

We will have to figure out all the library dependencies: I will write a script tomorrow which can find out all the dependencies.

✦ ❯ ldd venv/lib/python3.7/site-packages/PyQt5/QtWidgets.abi3.so 
	linux-vdso.so.1 (0x00007fff925bc000)
	libQt5Widgets.so.5 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libQt5Widgets.so.5 (0x00007f821d2c1000)
	libQt5Gui.so.5 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libQt5Gui.so.5 (0x00007f821c9a7000)
	libQt5Core.so.5 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libQt5Core.so.5 (0x00007f821c1cc000)
	libGL.so.1 => /lib/x86_64-linux-gnu/libGL.so.1 (0x00007f821c101000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f821c0e0000)
	libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f821bf5a000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f821bdd7000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f821bdbd000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f821bbfc000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f821b9de000)
	libicui18n.so.56 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libicui18n.so.56 (0x00007f821b545000)
	libicuuc.so.56 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libicuuc.so.56 (0x00007f821b18b000)
	libicudata.so.56 => /home/kdas/code/securedrop-client/venv/lib/python3.7/site-packages/PyQt5/Qt/lib/libicudata.so.56 (0x00007f82197a8000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f82197a3000)
	libgthread-2.0.so.0 => /lib/x86_64-linux-gnu/libgthread-2.0.so.0 (0x00007f821979e000)
	libglib-2.0.so.0 => /lib/x86_64-linux-gnu/libglib-2.0.so.0 (0x00007f821967f000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f821e23c000)
	libGLX.so.0 => /lib/x86_64-linux-gnu/libGLX.so.0 (0x00007f8219649000)
	libGLdispatch.so.0 => /lib/x86_64-linux-gnu/libGLdispatch.so.0 (0x00007f821958c000)
	libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007f8219518000)
	libX11.so.6 => /lib/x86_64-linux-gnu/libX11.so.6 (0x00007f82193d7000)
	libXext.so.6 => /lib/x86_64-linux-gnu/libXext.so.6 (0x00007f82191c5000)
	libxcb.so.1 => /lib/x86_64-linux-gnu/libxcb.so.1 (0x00007f821919b000)
	libXau.so.6 => /lib/x86_64-linux-gnu/libXau.so.6 (0x00007f8218f95000)
	libXdmcp.so.6 => /lib/x86_64-linux-gnu/libXdmcp.so.6 (0x00007f8218d8f000)
	libbsd.so.0 => /lib/x86_64-linux-gnu/libbsd.so.0 (0x00007f8218d75000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f8218d6b000)

[redshiftzero]

To @creviera's point, to install a later version of pyqt5 in sd-app so we don't need to maintain these workarounds, we could:

• Install upstream PyQt5 wheel as @kushaldas suggests. From their readme: "The wheels include a copy of the required parts of the LGPL version of Qt.". This would be distributed in the securedrop-app-code package and we'd remove qt-related debian dependencies. This would be a deviation from our current practice of building wheels ourselves however.
• Build and distribute via debian package the pyqt dependencies ourselves, and host on our apt server. This is more maintenance burden than I'd like.
• Build the PyQt5 wheel ourselves and include Qt in the wheel - this seems like the best option to me. We build the wheel ourselves so we can use whatever version of Qt+PyQt we want, and we'd still remove qt-related debian dependencies.

Thoughts?

[redshiftzero]

I was wondering about how - if we no longer use the buster pyqt5 package - we'd distribute python3-pyqt5.qtsvg (i.e. would we need to distribute a newer version of https://packages.debian.org/buster/python3-pyqt5.qtsvg and host on our own apt server?) but it looks like the pip-installable pyqt5 package has this functionality (e.g. if you look in the source tarball see sip/QtSvg/QtSvgmod.sip), so I don't think this is a problem.

[kushaldas]

I was wondering about how - if we no longer use the buster pyqt5 package - we'd distribute python3-pyqt5.qtsvg (i.e. would we need to distribute a newer version of https://packages.debian.org/buster/python3-pyqt5.qtsvg and host on our own apt server?) but it looks like the pip-installable pyqt5 package has this functionality (e.g. if you look in the source tarball see sip/QtSvg/QtSvgmod.sip), so I don't think this is a problem.

We are in luck, it seems the PyQt5 wheel contains the library for that too :)

[kushaldas]

For PyQt5.11.3

The dependencies look like:

{'elpa-yasnippet-snippets',
 'libasound2',
 'libasyncns0',
 'libatk-bridge2.0-0',
 'libatk1.0-0',
 'libatspi2.0-0',
 'libavahi-client3',
 'libavahi-common3',
 'libblkid1',
 'libbsd0',
 'libc6',
 'libcairo-gobject2',
 'libcairo2',
 'libcap2',
 'libcom-err2',
 'libcups2',
 'libdatrie1',
 'libdbus-1-3',
 'libdrm2',
 'libegl1',
 'libepoxy0',
 'libevdev2',
 'libexpat1',
 'libffi6',
 'libflac8',
 'libfontconfig1',
 'libfreetype6',
 'libfribidi0',
 'libgcc1',
 'libgcrypt20',
 'libgdk-pixbuf2.0-0',
 'libgl1',
 'libglib2.0-0',
 'libglvnd0',
 'libglx0',
 'libgmp10',
 'libgnutls30',
 'libgpg-error0',
 'libgraphite2-3',
 'libgssapi-krb5-2',
 'libgstreamer-plugins-base1.0-0',
 'libgstreamer1.0-0',
 'libgtk-3-0',
 'libgudev-1.0-0',
 'libharfbuzz0b',
 'libhogweed4',
 'libice6',
 'libidn2-0',
 'libinput10',
 'libk5crypto3',
 'libkeyutils1',
 'libkrb5-3',
 'libkrb5support0',
 'libldap-2.4-2',
 'libltdl7',
 'liblz4-1',
 'liblzma5',
 'libmount1',
 'libmtdev1',
 'libnettle6',
 'libnspr4',
 'libnss3',
 'libogg0',
 'liborc-0.4-0',
 'libp11-kit0',
 'libpango-1.0-0',
 'libpangocairo-1.0-0',
 'libpangoft2-1.0-0',
 'libpcre3',
 'libpixman-1-0',
 'libpng16-16',
 'libpq5',
 'libpulse-mainloop-glib0',
 'libpulse0',
 'libqt53danimation5',
 'libqt53dcore5',
 'libqt53dextras5',
 'libqt53dinput5',
 'libqt53dlogic5',
 'libqt53dquick5',
 'libqt53dquickscene2d5',
 'libqt53drender5',
 'libqt5gamepad5',
 'libqt5gui5',
 'libqt5multimediagsttools5',
 'libqt5multimediaquick5',
 'libqt5texttospeech5',
 'libqt5waylandclient5',
 'libqt5waylandcompositor5',
 'libqt5webview5',
 'libsasl2-2',
 'libselinux1',
 'libsm6',
 'libsndfile1',
 'libspeechd2',
 'libssl1.1',
 'libstdc++6',
 'libsystemd0',
 'libtasn1-6',
 'libthai0',
 'libudev1',
 'libunistring2',
 'libuuid1',
 'libvorbis0a',
 'libvorbisenc2',
 'libwacom2',
 'libwayland-client0',
 'libwayland-cursor0',
 'libwayland-egl1',
 'libwayland-server0',
 'libwrap0',
 'libx11-6',
 'libx11-xcb1',
 'libxau6',
 'libxcb-glx0',
 'libxcb-render0',
 'libxcb-shm0',
 'libxcb1',
 'libxcomposite1',
 'libxcursor1',
 'libxdamage1',
 'libxdmcp6',
 'libxext6',
 'libxfixes3',
 'libxi6',
 'libxinerama1',
 'libxkbcommon0',
 'libxrandr2',
 'libxrender1',
 'libxtst6',
 'zlib1g'}

For PyQt5.14.2 it looks like:

{'libaom0',
 'libasound2',
 'libasyncns0',
 'libatk-bridge2.0-0',
 'libatk1.0-0',
 'libatspi2.0-0',
 'libavahi-client3',
 'libavahi-common3',
 'libavcodec-extra58',
 'libavformat58',
 'libavutil56',
 'libblkid1',
 'libbluray2',
 'libbsd0',
 'libbz2-1.0',
 'libc6',
 'libcairo-gobject2',
 'libcairo2',
 'libcap2',
 'libchromaprint1',
 'libcodec2-0.8.1',
 'libcom-err2',
 'libcroco3',
 'libcrystalhd3',
 'libcups2',
 'libdatrie1',
 'libdbus-1-3',
 'libdrm2',
 'libegl1',
 'libepoxy0',
 'libevdev2',
 'libevent-2.1-6',
 'libexpat1',
 'libffi6',
 'libflac8',
 'libfontconfig1',
 'libfreetype6',
 'libfribidi0',
 'libgcc1',
 'libgcrypt20',
 'libgdk-pixbuf2.0-0',
 'libgl1',
 'libglib2.0-0',
 'libglvnd0',
 'libglx0',
 'libgme0',
 'libgmp10',
 'libgnutls30',
 'libgomp1',
 'libgpg-error0',
 'libgraphite2-3',
 'libgsm1',
 'libgssapi-krb5-2',
 'libgstreamer-plugins-base1.0-0',
 'libgstreamer1.0-0',
 'libgtk-3-0',
 'libgudev-1.0-0',
 'libharfbuzz0b',
 'libhogweed4',
 'libice6',
 'libicu63',
 'libidn2-0',
 'libinput10',
 'libjpeg62-turbo',
 'libk5crypto3',
 'libkeyutils1',
 'libkrb5-3',
 'libkrb5support0',
 'liblcms2-2',
 'libldap-2.4-2',
 'libltdl7',
 'liblz4-1',
 'liblzma5',
 'libminizip1',
 'libmount1',
 'libmp3lame0',
 'libmpg123-0',
 'libmtdev1',
 'libnettle6',
 'libnspr4',
 'libnss3',
 'libnuma1',
 'libodbc1',
 'libogg0',
 'libopenjp2-7',
 'libopenmpt0',
 'libopus0',
 'liborc-0.4-0',
 'libp11-kit0',
 'libpango-1.0-0',
 'libpangocairo-1.0-0',
 'libpangoft2-1.0-0',
 'libpcre3',
 'libpixman-1-0',
 'libpng16-16',
 'libpq5',
 'libpulse-mainloop-glib0',
 'libpulse0',
 'libqt53danimation5',
 'libqt53dcore5',
 'libqt53dextras5',
 'libqt53dinput5',
 'libqt53dlogic5',
 'libqt53dquick5',
 'libqt53dquickscene2d5',
 'libqt53drender5',
 'libqt5gamepad5',
 'libqt5gui5',
 'libqt5multimediaquick5',
 'libqt5test5',
 'libqt5texttospeech5',
 'libqt5webengine5',
 'libqt5webenginecore5',
 'libqt5webview5',
 'libre2-5',
 'librsvg2-2',
 'libsasl2-2',
 'libselinux1',
 'libshine3',
 'libsm6',
 'libsnappy1v5',
 'libsndfile1',
 'libsoxr0',
 'libspeechd2',
 'libspeex1',
 'libssh-gcrypt-4',
 'libssl1.1',
 'libstdc++6',
 'libswresample3',
 'libsystemd0',
 'libtasn1-6',
 'libthai0',
 'libtheora0',
 'libtwolame0',
 'libudev1',
 'libunistring2',
 'libuuid1',
 'libva-drm2',
 'libva-x11-2',
 'libva2',
 'libvdpau1',
 'libvorbis0a',
 'libvorbisenc2',
 'libvorbisfile3',
 'libvpx5',
 'libwacom2',
 'libwavpack1',
 'libwayland-client0',
 'libwayland-cursor0',
 'libwayland-egl1',
 'libwebp6',
 'libwebpdemux2',
 'libwebpmux3',
 'libwrap0',
 'libx11-6',
 'libx11-xcb1',
 'libx264-155',
 'libx265-165',
 'libxau6',
 'libxcb-glx0',
 'libxcb-render0',
 'libxcb-shm0',
 'libxcb-xkb1',
 'libxcb1',
 'libxcomposite1',
 'libxcursor1',
 'libxdamage1',
 'libxdmcp6',
 'libxext6',
 'libxfixes3',
 'libxi6',
 'libxinerama1',
 'libxkbcommon-x11-0',
 'libxkbcommon0',
 'libxml2',
 'libxrandr2',
 'libxrender1',
 'libxslt1.1',
 'libxss1',
 'libxtst6',
 'libxvidcore4',
 'libzvbi0',
 'zlib1g'}

Even though they show dependencies to libqt5* packages, it seems they are from the Qt plugins which don't get loaded generally.

I used the following script https://gist.github.com/kushaldas/d1e639d58eb86191d2485de0c6a59a0a

[sssoleileraaa]

I came across this issue during backlog grooming, and had to reread to jog my memory. Here's what we're waiting on and why this issue is still open:

• the upstream bug has been fixed but sd-app is on Buster which comes with PyQt 5.11.3, a version of PyQt that doesn't contain the fix
• Since Move CSS to CSS files and add new tests for stylesheets #1082 was merged, we now avoid hitting this bug by limiting where we set widget stylesheets (it could be encountered if new css files or new inline CSS is added without considering the details of how this bug can rear its ugly head, for more info see QTBUG-69204 - Upstream Qt stylesheet bug causes segmentation faults #273 (comment) and see securedrop_client/resources/css for how we organize CSS)
• We want to install a later version of PyQt in sd-app so that we don't need to worry about this upstream bug anymore. Right now we are working on add more developer docs #1089, which is a PR for learning/sharing how to build Qt from source so that we can select which versions of PyQt+Qt we use and test against

[kushaldas]

If anyone has time to investigate further, here is another way you can reproduce this upstream bug:

app = QApplication(qt_args)
w = QMainWindow()
conversation = QWidget()
conversation_layout = QVBoxLayout()
conversation.setLayout(conversation_layout)
scroll_widget = QWidget()
scroll_widget_layout = QVBoxLayout()
scroll_widget.setLayout(scroll_widget_layout)
scroll = QScrollArea()
scroll.setWidget(scroll_widget)
conversation_layout.addWidget(scroll)
message = QLabel('hey')
message.setTextInteractionFlags(Qt.TextSelectableByMouse)
conversation.setStyleSheet('a { b:c; }')
scroll_widget_layout.addWidget(message)
w.setCentralWidget(conversation)
w.show()
sys.exit(app.exec_())

In today's Debian Buster, we don't see any more segfault. Some magic happened somewhere in the last few months.