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

Fix 'undefined behaviors' detected by MIRI #17

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

Conversation

arthurprs
Copy link

I found the following error on the Miri report when I was running Miri on something else.

The shortest fix is to make the fields cover the null byte as well. I didn't find any APIs that exposed bytes to the user so this change should be backwards compatible.

test test::read_pid ... error: Undefined Behavior: attempting a read access using <294210> at alloc102399[0x17], but that tag does not exist in the borrow stack for this location
   --> src/unix.rs:266:9
    |
266 | /         libc::open(
267 | |             path.bytes.as_ptr(),
268 | |             libc::O_RDWR | libc::O_CLOEXEC | libc::O_CREAT,
269 | |             (libc::S_IRUSR | libc::S_IWUSR | libc::S_IRGRP | libc::S_...
270 | |                 as libc::c_int,
271 | |         )
    | |         ^
    | |         |
    | |_________attempting a read access using <294210> at alloc102399[0x17], but that tag does not exist in the borrow stack for this location
    |           this error occurs as part of an access at alloc102399[0x17..0x18]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <294210> was created by a SharedReadOnly retag at offsets [0x0..0x17]
   --> src/unix.rs:267:13
    |
267 |             path.bytes.as_ptr(),
    |             ^^^^^^^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span) on thread `test::read_pid`:
    = note: inside `unix::open` at src/unix.rs:266:9: 271:10
note: inside `LockFile::open::<str>`
   --> src/lib.rs:123:20
    |
123 |         let desc = sys::open(path.as_ref())?;

@@ -228,7 +232,8 @@ fn make_os_str(slice: &[u8]) -> Result<EitherOsStr, Error> {
panic!("Path to file cannot contain nul-byte in the middle");
}
if last == 0 {
let str = unsafe { OsStr::from_slice(transmute(slice)) };
let str =
Copy link
Author

@arthurprs arthurprs Aug 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice how the Borrowed slice includes the null byte in this block, arguably incorrect before.

Now, that will be intentional. The typed transmute is to appease cargo clippy.

@arthurprs arthurprs marked this pull request as ready for review August 18, 2024 16:54
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.

1 participant