Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

passing empty wayland raw window handle segfaults in safe code #78

Closed
m4b opened this issue Sep 9, 2019 · 6 comments
Closed

passing empty wayland raw window handle segfaults in safe code #78

m4b opened this issue Sep 9, 2019 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@m4b
Copy link
Contributor

m4b commented Sep 9, 2019

Using wgpu 0.3.0,

let handle = raw_window_handle::RawWindowHandle::Wayland(raw_window_handle::unix::WaylandHandle::empty());
let surface = _instance.create_surface(handle);

This will cause a segfault in safe code.

@grovesNL
Copy link
Collaborator

grovesNL commented Sep 9, 2019

Right, we should be null checking here if anyone would like to take a look at improving this.

@grovesNL grovesNL added bug Something isn't working help wanted Extra attention is needed labels Sep 9, 2019
@kvark
Copy link
Member

kvark commented Sep 9, 2019

I don't know if we can even truly address this. Users can always provide garbage pointers in RawWindowHandle. It looks like the only true way to solve this is to turn create_surface() to be unsafe.

@HeroesGrave
Copy link

As per the HasRawWindowHandle docs:

It is entirely valid behavior for fields within each platform-specific RawWindowHandle variant to be null or 0, and appropriate checking should be done before the handle is used. However, users can safely assume that non-null/0 fields are valid handles, and it is up to the implementor of this trait to ensure that condition is upheld.

It should be enough to just check for null/0.

@Lokathor
Copy link

create_surface can remain safe if it just checks for null and then panics. It's obviously not ideal, but it's always legal for safe code to panic.

@pythonesque
Copy link

We also came to the same conclusion (this interface can be made safe as long as we check for 0/null on all the fields we need, in a platform-dependent way). I think it would be best to do this check directly in gfx-rs since we know exactly which handles we need at any given time. So I might take a stab at this (it would also be much better if this returns a Result rather than panicking; we ran into issues with gfx-rs panicking on "obvious" errors that we couldn't easily catch at runtime, and I think almost anyone would trade having to handle a Result for safety).

@kvark
Copy link
Member

kvark commented Jun 3, 2021

Closing in favor of #674

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants