Skip to content

Conversation

@TheBlueMatt
Copy link

When hot-plugging monitor(s) ipc_json_describe_workspace can end up dereferencing a NULL ptr in wlr_output->name. Here we simply check that it is set.

While this is probably a race and just checking that it is not NULL before accessing it won't fully solve the race, it should make it substantially more rare.

Mostly addresses #8747

When hot-plugging monitor(s) `ipc_json_describe_workspace` can end
up dereferencing a NULL ptr in `wlr_output->name`. Here we simply
check that it is set.

While this is probably a race and just checking that it is not NULL
before accessing it won't fully solve the race, it should make it
substantially more rare.

Mostly addresses swaywm#8747
@emersion
Copy link
Member

I would prefer to find the root cause of this NULL pointer instead of adding a workaround.

@TheBlueMatt
Copy link
Author

Sure, don't disagree, but absent someone having time to dig into it probably worth at least making sway stable.

@TheBlueMatt
Copy link
Author

Any update here?

@emersion
Copy link
Member

emersion commented Nov 3, 2025

Would still prefer to find the root cause rather than merge this.

@TheBlueMatt
Copy link
Author

If no one has time to dig into the root cause it still might presumably be preferrable to merge this so that at least sway is stable on impacted devices. A broken sway seems worse than a workaround for a bug.

@emersion
Copy link
Member

emersion commented Nov 3, 2025

If a workaround is merged, it removes any incentive to find the root cause.

@TheBlueMatt
Copy link
Author

If impacted users migrate off of sway, it also removes any incentive to fix the issue :p

@kennylevinsen
Copy link
Member

If impacted users migrate off of sway, it also removes any incentive to fix the issue :p

The fact remains that adding a NULL check for a field that can never be NULL isn't a fix. The value should be set by the backend at the very start, and when destroyed it is simply free'd, not NULL'd.

It could possibly be a use-after-free/dangling pointer of sway_output or wlr_output, rather than being related to the name field itself. Reproducing the issue with address sanitizer enabled might help pinpoint things. That is done by specifying -Db_sanitize=address to meson, and will cause a report to be logged to stderr when a memory snafu is detected, and might give a clue as to what's happening.

(I don't have a reproduction of this issue myself.)

@njdom24

This comment was marked as off-topic.

@emersion
Copy link
Member

emersion commented Nov 11, 2025

@njdom24, that sounds like a different issue. My guess would be that con->pending.workspace is NULL.

@deep4lpha

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants