Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StackCheckedPointer is unsound #84

Closed
kevinmehall opened this issue Jan 2, 2019 · 3 comments
Closed

StackCheckedPointer is unsound #84

kevinmehall opened this issue Jan 2, 2019 · 3 comments

Comments

@kevinmehall
Copy link

The documentation describes an assumption Azul's data binding relies on when it bypasses the borrow-checker:

The pointer is inside the boundaries of T to T + size_of::<T>() - as long as T is alive, the pointer points to valid memory.

Unfortunately, nothing in Rust's type system guarantees this. The most obvious counterexample is enum types.

Here's an example showing how code without unsafe can cause an invalid pointer dereference and segfault using the safe interface of TextInput:
https://gist.github.com/kevinmehall/44190fea3e4775c90d175b2bb6c07e53
(this example could do without the RefCell if it were mutated elsewhere, in a callback or something).

@fschutt
Copy link
Owner

fschutt commented Jan 2, 2019

Very good point, thanks for reporting. Yes, it is unsound and there currently seems to be no way of preventing this other than common sense (not to modify the data model within the layout() function). To "fix" this, it might be possible to store extra information such as enum discriminant ID and TypeId of the original type, to at least safeguard better against these issues (for example, a trait that could report the sizes, offsets and enum IDs automatically from the compiler at compile time, then check whether the pointer is contained inside of a RefCell or whether a callback has modified the enum variant). Sort of like RTTI, but without type information, only size and offset information.

What basically needs to happen is some kind of check in the layout function whether the pointer that the callback has acted on could be or has been modified from the time of creation to the time of invokation.

@o01eg
Copy link

o01eg commented Mar 11, 2019

Could it be implemented via Pin?

@tobz1000
Copy link

tobz1000 commented Apr 2, 2019

As I understand it, Pin is a generalisation of what StackCheckedPointer is trying to achieve - and would allow previously-proven self-referential structures to be used where possible.

Is there any reason not to allow users to provide a &mut TextInputState-getter function, rather than a direct reference to the field; e.g. textInput.bind(window, |model| &mut model.text_input, &mut model)? I imagine the safety gain would be worth the small performance hit in many instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants