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

Replace transmuted references with explicit pointer type casts #308

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickbeth
Copy link

Description

Rust docs about std::mem::transmute read:

Transmuting integers to pointers is a largely unspecified operation. It is likely not equivalent to an as cast. Doing non-zero-sized memory accesses with a pointer constructed this way is currently considered undefined behavior.

Source: https://doc.rust-lang.org/std/mem/fn.transmute.html#transmutation-between-pointers-and-integers

Although this library mostly transmutes to reference Dst types and not pointers, from my understanding the same rules apply for references as well. Additionally, code in the library sometimes transmutes to mutable references, which is even worse because strict aliasing rules may not be respected in that case.

Motivation

An issue was opened for my app, a Win32 GUI for WSL USB device management, reporting that release builds would crash at startup, with a STATUS_ILLEGAL_INSTRUCTION return code.
Upon further inspection, the issue boiled down to these transmutes [1] [2] in the tabs code. In that specific instance, UB was introduced, and for any opt-level > 0 the generated code would callq into a panic handler without a conditional jump before it, return from the handler and execute an ud2 instruction.

This was tested on Windows 11, Rust 1.82.0.

Change proposal

std::mem::transmute instances that would introduce UB are removed in favor of explicit type casts, and references are changed to pointers. There's many instances of std::mem::transmute being used incorrectly, this PR fixes all offending instances.

This commit replaces all usages of `std::mem::transmute` on references in the codebase with explicit pointer type casts.
This is done to prevent undefined behavior caused by transmuting integer types to references, and subsequently dereferencing them.
@JPHutchins
Copy link

May be some overlap in this PR: #250

@nickbeth
Copy link
Author

Thanks @JPHutchins. It doesn't look like there should be any overlaps between the two PRs, as the changes are on different files. Also the impact of the other PR is none apart from code style changes.

@nickbeth
Copy link
Author

Fixes #310.

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

Successfully merging this pull request may close these issues.

2 participants