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(uhyve): fix improper unlink string handling #1503

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

Conversation

n0toose
Copy link
Member

@n0toose n0toose commented Dec 8, 2024

Taken from traverse_open.

@n0toose
Copy link
Member Author

n0toose commented Dec 9, 2024

I caught this as I was working on benchmarks for Uhyve. Using a modified test of Uhyve, this seems to be working. There was an error when converting a char pointer to an str because it was not in UTF-8 - which is a check that I built into Uhyve to prevent weird behavior like this. This means that fs::remove_file has been broken on Uhyve for a while now - but I did not catch that, as I tested the call on my host system without a UhyveVm under the assumption that it previously did.

Anyways, this is not the case anymore. CC: @jounathaen

[0][INFO] Try to initialize uhyve filesystem
[0][INFO] Mounting uhyve filesystem at /root
[0][INFO] Jumping into application
Hello from create_file!
Successfully removed file!
Number of interrupts
[0][FPU]: 1

@n0toose n0toose changed the title bug(uhyve): fix improper unlink string handling fix(uhyve): fix improper unlink string handling Dec 9, 2024
@n0toose
Copy link
Member Author

n0toose commented Dec 9, 2024

Here's the code to reproduce, @mkroening:

use std::{fs::File, io::prelude::*, fs::remove_file};

#[cfg(target_os = "hermit")]
use hermit as _;

fn main() {
	println!("Hello from create_file!");

	let mut file = File::create("/root/foo.txt").unwrap();
	file.write_all(b"Hello, world!").unwrap();
        // this is what I added to the test myself
	remove_file("/root/foo.txt").unwrap();
	println!("Successfully removed file!");
}

Seems like this "bug" was caused by the following commit: ba9f92c

... which took some extra steps to ensure that null terminators were handled properly in traverse_open but were perhaps forgotten about in traverse_unlink, which was unused at the time. The unlink call at the time accepted any path on the host OS and was / is not 'mount-aware' the same way open is.

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