Skip to content

fix: check if resource is good before accessing it in DnD functions#13852

Draft
StevenArai wants to merge 1 commit intohyprwm:mainfrom
StevenArai:fix-dnd-crash-use-after-free
Draft

fix: check if resource is good before accessing it in DnD functions#13852
StevenArai wants to merge 1 commit intohyprwm:mainfrom
StevenArai:fix-dnd-crash-use-after-free

Conversation

@StevenArai
Copy link
Copy Markdown

Prevent use-after-free crash when accessing wayland resources that may have been destroyed. Add good() checks to all CWLDataSourceResource methods that access m_resource directly.

Fixes crash in sendDndFinished() when called from forceCleanupDnd() during xwayland dnd cleanup.

Describe your PR, what does it fix/add?

this pr fixes a crash in dnd code. The crash happens when im dragging a file from GNOME file manager (nautilus) to vscode and it brings me to the safe mode.

I add good() checks to all CWLDataSourceResource methods. These checks make sure the resource is valid before anyone could use it.

the crash was in sendDndFinished() when called from forceCleanupDnd() during xwayland dnd cleanup.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

The crash happens sometimes when stopping DnD operations with XWayland apps. The backtrace shows:
wl_resource_get_version()
CWLDataSourceResource::sendDndFinished()
CX11DataDevice::forceCleanupDnd()
CWLDataDeviceProtocol::abortDrag()

It's not replicatable stably on my environment, so im having trouble doing a further test over it.

Is it ready for merging, or does it need work?

I want someone with same problem to have a test for me, if my work isnt pure garbage(

sorry for bad English :(

Prevent use-after-free crash when accessing Wayland resources that may
have been destroyed. Add good() checks to all CWLDataSourceResource
methods that access m_resource directly.

Fixes crash in sendDndFinished() when called from forceCleanupDnd()
during XWayland DnD cleanup.
@github-actions
Copy link
Copy Markdown

Hello and thank you for making a PR to Hyprland!

Please check the PR Guidelines and make sure your PR follows them.
It will make the entire review process faster. :)

If your code can be tested, please always add tests. See more here.

beep boop, I'm just a bot. A real human will review your PR soon.

@StevenArai
Copy link
Copy Markdown
Author

clang-format check didnt pass...
wait im figuring this out

@vaxerski
Copy link
Copy Markdown
Member

rebase on main and run clang-format, should be ok

@StevenArai
Copy link
Copy Markdown
Author

rebase on main and run clang-format, should be ok

it isnt my code not passing

@vaxerski
Copy link
Copy Markdown
Member

yes thats why rebase on main

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants