rust-random / getrandom

A small cross-platform library for retrieving random data from (operating) system source
Apache License 2.0
264 stars 166 forks source link

Bump MSRV to 1.57 & improve the memory safety guarantees around `open_readonly`. #432

Closed briansmith closed 1 month ago

briansmith commented 1 month ago

Do more complete NUL termination checking, at compile-time. Remove the unsafe from the function as it is now memory-safe.

josephlr commented 1 month ago

As I wrote in your other PR, I don't think it's worth to introduce the Ref type.

A better solution would be to use zero-terminated byte slices (i.e. const FILE_PATH: &[u8] = b"/dev/urandom\0";) for paths and add the following asserts to open_readonly:

I agree. This is a lot of complexity to just ensure that a byte slice passed to a function contains a zero byte (which is all we need for safety).

assert_eq!(path[path.len() - 1], 0);
assert!(path[..path.len() - 1].iter().all(|&b| b != 0));

Since we use static paths, compiler should be able to optimize them out.

I think the assert just needs to be:

assert!(path.iter().any(|&b| b == 0));

for the function to be safe. If our slice has bytes after the null-terminator, that's weird but not unsafe. This implementation has the advantage of optimizing out at even -C opt-level=1: https://rust.godbolt.org/z/h9EaPnaaY

newpavlov commented 1 month ago

Closing in favor of #434.