rust-lang / libc

Raw bindings to platform APIs for Rust
https://docs.rs/libc
Apache License 2.0
2.08k stars 1.04k forks source link

Make functions which won't cause UB for any input safe #2895

Closed HKalbasi closed 1 month ago

HKalbasi commented 2 years ago

That is, for example, calling sleep is always fine, with any input, so we can add a #[inline(always)] extern "C" safe wrapper on it, and expose that instead of the original unsafe variant. Similarly, calling malloc on it's own is always ok, only using the result pointer might cause UB, which needs unsafe anyway, so malloc can become safe as well. On the other hand, calling free with bad pointer is UB, so free should remain unsafe.

It helps making unsafe blocks smaller in libc heavy codes, and makes people pay more attention in using really unsafe functions.

fogti commented 2 years ago

You might want to rephrase your issue title, by replacing "all" with "any" to avoid confusion.

workingjubilee commented 1 year ago

The claim that a bare call to malloc is always safe is inaccurate. It is not enough to merely be non-UB for all inputs. In particular, malloc is not required to have a thread-safe implementation by C, nor any of the other things that are generally imposed as burdens by default on Rust code. The C Standard is largely mute on questions of thread safety, last I checked, aside from noting that data races are in fact UB and that synchronization events can occur e.g. by defining _Atomic. It is required to have thread safety by POSIX, but not all systems are POSIX, and of the ones that claim to implement POSIX, not all implement it correctly.

It may be acceptable to export safe wrappers for some functions for some targets, but we must heavily scrutinize each and every case, unfortunately. Because replacing malloc is very common, I think it is especially dubious, as some allocators use global structures instead of thread-local ones. Granted, we arguably are exposed directly to any potential raciness or unsoundness by the stdlib's implementation, but the stdlib also can interpose its own code to guarantee safety without breaking backwards compatibility, and libc definitely should not be interposing unnecessarily wrapping Rust code. It should expose the naughtiness of the underlying system. Directly. That means it has a high risk of being unsafe, even for functions which seem "obviously safe".

I would consider an "obviously safe" function to be more like gettimeofday, where it may return a bad or incoherent result, but it's probably not going to race on a global data structure (I hope) (maybe). Even then, do we really want to expose safe wrappers that would only apply to some systems? Do we really want to replay the setenv fiasco again?

tgross35 commented 1 month ago

I have to agree with Jubilee here. Plenty of these files "look" safe but it's totally implementation dependent, and require reading both the docs and the source to ensure that it doesn't crash your program or worse. We can't reasonably provide that required level of scrutiny to libc, especially given how many platforms are supported and that the vast majority of functions in this library can be used to cause UB in some way.

Closing this because we don't want to provide any guarantees here, this is a very much an interface-only "you need to read your system's docs" crate. nix provides safe wrappers for many Linux/Unix APIs, if that better suits your needs.