Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Comparative unsafety

flak.tedunangst.com

Post reply on HN
Print view View original

61–70 of 99 posts

Re: Comparative unsafety

#61

Post by MaulingMonkey »

im3w1l wrote: post #23
einpoklum wrote: post #2

> I wrote some rust code. Actually, he wrote some Rust and some C code wrapped in Rust. > I was writing an smtp server... the kind of smtp server you'd write in two days. (sigh) > The error would have been detected far sooner had I not been lazy and checked the return value of chown (for a file not found error). But [excused here] You can't, and mustn't avoid C error checking when writing C code, even if it's wrapped…

> I was a TA in a first-semester course in C programming for several years, and one of the harder things to inculcate is how return value checking is not optional. It's very tempting to assume your standard library function just works, always. The situation is worse than that. People check for errors except when they don't. And you are expected to know the difference. Take for instance malloc. It can return NULL but…

> Take for instance malloc. It can return NULL but you don't check for it, because that's just the way it's done.

malloc is worth null-checking, if only to assert/panic/breakpoint/fatal early and cleanly.

Even on linux with overcommit, it'll return null on memory space exhaustion, which can easily happen even in a 64-bit processes if someone feeds your program maliciously crafted data with an oddball size. A crash dump at the point of allocation failure, when the sizes are still on the call stack / in registers, instead of minutes later - when the pointer is actually used - is also a vastly better debugging experience when fuzzing for such bugs.

If malicious data can control offsets into the (null) pointer, an allocation failure can turn into an exploitable buffer overflow (just make the offset large enough), even if the offset is bounds-checked against the "expected" (even larger) allocation size.

Re: Comparative unsafety

#62

Post by iainmerrick »

cesarb wrote: post #32
howeyc wrote: post #12

Wait, what? I don't know rust, at all, but why is path freed? I thought rust didn't have GC? > let path = CString::new(filename.as_str()).unwrap().as_ptr(); Is one of those functions calling free behind the scenes?

It's freed for the same reason it would be freed in C++. Like C++, Rust relies heavily on RAII, in which resources are freed once they get out of scope. The "CString::new(filename.as_str()).unwrap()" returns a CString, which owns the memory for the C string. Since it's a temporary (it's not being stored anywhere), its scope ends before the next line, so the resources it owns are freed (by calling its Drop implementat…

So shouldn’t .as_ptr() itself be marked unsafe? That way only the correct version of this code, with the .as_ptr() inside the unsafe block, would compile, right?

(I assume there’s a good reason why this isn’t the case, but I don’t know Rust well enough to know the answer. I’d be interested to learn, if anyone can explain it for me!)

Edit to clarify: what seems very strange to me is that a function that not only does something potentially unsafe, but returns an unsafe value would be callable outside of an “unsafe” block.

Re: Comparative unsafety

#63

Post by nyanpasu64 »

cesarb wrote: post #43
nicoburns wrote: post #8

From my perspective as someone who knows Rust but primarily works with high-level languages like JavaScript, some developers who from a background of unsafe languages (particularly C and C++) seem incredibly cavalier around `unsafe` blocks in Rust code. It's like they're desensitised to the unsafety. As someone who is used to working in a language where one absolutely cannot hit memory safety issues or undefined beha…

> From my perspective as someone who knows Rust but primarily works with high-level languages like JavaScript, some developers who from a background of unsafe languages (particularly C and C++) seem incredibly cavalier around `unsafe` blocks in Rust code. It's like they're desensitised to the unsafety. That's true. If you are used to 100% of your code being within the equivalent of a Rust "unsafe" block, having over…

> Even if one in ten lines are marked as "unsafe", that's already many times less "unsafe" than what they are used to in their C or C++ code.

As a Rust programmer who's written safe code and some unsafe abstractions, one in ten lines being unsafe means that the remaining 90% of your lines need to be written with the same care as your unsafe code, to avoid feeding invalid arguments into unsafe operations which cause UB. The general approach is to not scatter unsafe code around your entire program, but segregate it into safe abstractions that prevent callers from triggering UB. (Though some APIs make it difficult to achieve, so the alternative is to create unsafe abstractions that expect callers to take care to avoid triggering UB, which requires auditing the callers for security as well.)

Re: Comparative unsafety

#64

Post by yakubin »

iainmerrick wrote: post #62
cesarb wrote: post #32

Earlier quoted context omitted.

It's freed for the same reason it would be freed in C++. Like C++, Rust relies heavily on RAII, in which resources are freed once they get out of scope. The "CString::new(filename.as_str()).unwrap()" returns a CString, which owns the memory for the C string. Since it's a temporary (it's not being stored anywhere), its scope ends before the next line, so the resources it owns are freed (by calling its Drop implementat…

So shouldn’t .as_ptr() itself be marked unsafe? That way only the correct version of this code, with the .as_ptr() inside the unsafe block, would compile, right? (I assume there’s a good reason why this isn’t the case, but I don’t know Rust well enough to know the answer. I’d be interested to learn, if anyone can explain it for me!) Edit to clarify: what seems very strange to me is that a function that not only does…

I don't see anything unsafe here. Merely storing a pointer, whatever it may be, doesn't violate memory-safety. Dereferencing a pointer in some cases does. The article contains one of those cases: use-after-free. If the pointer wasn't dereferenced, all code would be fine, including the line with ".as_ptr()".

Re: Comparative unsafety

#65

Post by pornel »

wyldfire wrote: post #6

I will concede that it would be nice if I could opt-in to a 'be-really-nitpicky' mode for clippy. I would even do it sometimes. Usually I just want something with a little higher SNR.

Good news!

    cargo clippy -- -W clippy::pedantic
or `#![warn(clippy::pedantic)]` in your crate.

Here's a list of lints and groups you can turn on/off: https://rust-lang.github.io/rust-clippy/master/index.html

Re: Comparative unsafety

#66

Post by nyanpasu64 »

Fun fact: I hit the exact same bug when working on FamiTracker in C++, with a MFC type also called CString... "construct a CString, acquire a pointer into the internals, destroy the CString". The only reason it doesn't make FamiTracker crash is because MFC's CString is COW/refcounted, and the refcount never reached zero because it was constructed from a long-lasting string literal, so the underlying buffer was still valid after destroying the string object.

I feel that tying deallocations to destructors is a benefit to safe code, and a hindrance to unsafe code. I feel explicit calls to "destructor" functions that move self (perhaps using defer), plus linear typing to statically ensure you don't forget to call a destructor or call one twice, can provide safety in both situations, but at the expense of verbosity and not interacting well with exceptions. I still believe linear typing (argued against by Gankra in https://gankra.github.io/blah/linear-rust/) will fix multiple deficiencies in Rust:

- It helps write allocation-free code which passes objects around without creating/destroying them (maybe an Alloc effect would help too).

- Making drops explicit makes it harder to screw up when writing unsafe code performing allocations/deallocations. It may or may not help protect against double-frees that occur upon panicking.

- File's Drop implementation ignores the return code of fclose(), since you can't return values from a destructor. So dropping a File directly should be prohibited (except during panic-unwinding, which I feel is a rare scenario where perfect error-checking is less crucial than not risking more panics). Instead you move the file into a close() method/function, and are expected to check the Result that comes out.

- From what I've read, trying to perform an asynchronous cancellation operation in the standard Drop method is not possible, since the method lacks access to the executor (unless you hard-code an executor to block on). It would make sense to make explicit dropping illegal (as a lint) and require moving it into a "drop"-like function, but keep Drop around to be called during panic-unwinding. This may result in logically incorrect behavior, but is still memory-safe. And I think it's better than making your async-fn state machines always larger to fit awaiting-a-drop states, and awaiting the async runtime when you panic-unwind.

Re: Comparative unsafety

#68

Post by pornel »

jdc wrote: post #59
rectang wrote: post #37

In Rust, dereferencing a pointer is unsafe and so must be isolated within an `unsafe` block — but creating a dangling pointer is not unsafe. What this means is that as soon as you add `unsafe` code which accepts a pointer argument, all the supposedly "safe" code which can affect this pointer argument becomes a potential source of memory errors. To me, this was unintuitive — I expected the `unsafe` block to require an…

I wonder what a good approach to safe pointer arithmetic would be.

Rust's actually-safe pointer arithmetic is done via slices (because it's necessary to track the length):

    slice = &slice[1..];
It compiles to roughly `ptr++; len--;` and a bounds check if necessary.

`1234 as *const u8` and `ptr.offset(n)` are also safe, because Rust allows invalid/dangling pointers to exist if they're never dereferenced.

Re: Comparative unsafety

#69

Post by rectang »

nyanpasu64 wrote: post #66

Fun fact: I hit the exact same bug when working on FamiTracker in C++, with a MFC type also called CString... "construct a CString, acquire a pointer into the internals, destroy the CString". The only reason it doesn't make FamiTracker crash is because MFC's CString is COW/refcounted, and the refcount never reached zero because it was constructed from a long-lasting string literal, so the underlying buffer was still…

> File's Drop implementation ignores the return code of fclose()

Yeah, and it bothers me that I can't call `close` and check for errors. All you need is something like this:

    pub fn close(self) -> io::Result {
        let status = unsafe { libc::close(self.fd) };
        mem::forget(self); // prevent drop and invalid double-close
        match status {
            0 => Ok(()),
            _ => Err(io::Error::new(ErrorKind::Other)),
        }
    }
Note that this function takes `self` not `&mut self`, so it consumes the resource and prevents further use. Then `mem::forget` prevents `drop` from running.

See discussion at https://github.com/rust-lang/rust/issues/59567

Re: Comparative unsafety

#70

Post by im3w1l »

MaulingMonkey wrote: post #61
im3w1l wrote: post #23

Earlier quoted context omitted.

> I was a TA in a first-semester course in C programming for several years, and one of the harder things to inculcate is how return value checking is not optional. It's very tempting to assume your standard library function just works, always. The situation is worse than that. People check for errors except when they don't. And you are expected to know the difference. Take for instance malloc. It can return NULL but…

> Take for instance malloc. It can return NULL but you don't check for it, because that's just the way it's done. malloc is worth null-checking, if only to assert/panic/breakpoint/fatal early and cleanly. Even on linux with overcommit, it'll return null on memory space exhaustion, which can easily happen even in a 64-bit processes if someone feeds your program maliciously crafted data with an oddball size. A crash du…

That's a very interesting idea for exploitation, I bet a lot of things are vulnerable to that.

But anyway I doubt that people do it even though they should.

Post reply on HN