wlserver: check wlr_xwayland_server_create for failure - #2337
Open
adamsjo95 wants to merge 1 commit into
Open
Conversation
wlr_xwayland_server_create() returns NULL on failure, but its return value was used unchecked. When Xwayland server creation fails, the following wl_signal_add() dereferences the NULL pointer and gamescope segfaults with no indication of what went wrong. wlroots' own caller in xwayland/xwayland.c checks the return value. Log the failure and abort instead, matching the handling of the wlr_output_commit_state() failure below.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wlr_xwayland_server_create()returns NULL on failure, butgamescope_xwayland_server_t's constructor uses the return valueunchecked, so the next line dereferences NULL and gamescope segfaults
with no indication of the cause.
wlroots' own caller checks it (
xwayland/xwayland.c), and the functionhas four
return NULLpaths — missing Xwayland binary, allocationfailure, and two error labels.
Reproduction
Any condition that makes Xwayland server creation fail will trigger it.
I hit it with a
/tmp/.X11-unixthat wlroots rejects on ownershipgrounds, which exhausts all 33 display slots:
Backtrace before the patch:
signal=0x90is the offset ofevents.readyon a NULL base.After
Exit status goes from 139 (SIGSEGV) to 134 (SIGABRT), with the cause
named.
Notes
I used
abort()to match thewlr_output_commit_state()failurehandling twelve lines below in the same constructor. Happy to change it
to a cleaner exit if preferred — this failure is often a user-fixable
environment problem rather than an internal invariant violation, so a
non-abort path may read better.
Tested on gamescope
fcc1341, wlroots 0.20.2. Builds clean with--werror.meson test --suite gamescopepasses 2/2 before and after.