Closed tiif closed 1 month ago
@rustbot ready
Thanks!
As just mentioned in the issue, this needs two new "fail-dep" tests: a double free of malloc(0), and a use after free.
I am trying to create a use-after-free test, but the test below surprisingly not throwing any error?
unsafe {
let ptr = libc::malloc(0);
libc::free(ptr);
let _ = *ptr;
}
Yes, that's how let _ =
behaves. It creates the place but doesn't read from it. And yes this is surprising. :) We should really document this better... somewhere...
Use let _val = *ptr;
.
I see, right now this error shows up, how is pointer dereference related to Copy?
error[E0507]: cannot move out of `*ptr` which is behind a raw pointer
--> ./tests/fail-dep/libc/malloc_zero_use_after_free.rs:5:20
|
5 | let _val = *ptr;
| ^^^^ move occurs because `*ptr` has type `libc::c_void`, which does not implement the `Copy` trait
|
help: consider removing the dereference here
|
5 - let _val = *ptr;
5 + let _val = ptr;
|
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0507`.
error: test failed, to rerun pass `--test ui`
full test:
fn main() {
unsafe {
let ptr = libc::malloc(0);
libc::free(ptr);
let _val = *ptr;
}
}
c_void is not Copy so you cannot just do a deref.
But anyway this will not give the desired error as ZST accesses are NOPs. I should not have said use after free, sorry. I meant memory leak and double free.
c_void is not Copy so you cannot just do a deref. But anyway this will not give the desired error as ZST accesses are NOPs. I should not have said use after free, sorry. I meant memory leak and double free.
Ah I see, thanks!
@rustbot author
There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.
You can start a rebase with the following commands:
$ # rebase
$ git pull --rebase https://github.com/rust-lang/miri.git master
$ git push --force-with-lease
The following commits are merge commits:
@rustbot ready
Awesome. :) Please squash the commits.
@bors r+
:pushpin: Commit 3a2524ab572ce9e3dadebe9a05d13dbc69120453 has been approved by RalfJung
It is now in the queue for this repository.
:hourglass: Testing commit 3a2524ab572ce9e3dadebe9a05d13dbc69120453 with merge 6714e03fcf3a5dcfb6002e525ddf29a294ad84eb...
:sunny: Test successful - checks-actions Approved by: RalfJung Pushing 6714e03fcf3a5dcfb6002e525ddf29a294ad84eb to master...
I just realized this also affects the Windows HeapAlloc function. @ChrisDenton I didn't find anything in the docs about HeapAlloc with size 0... is it okay for that to return a non-null pointer that must be freed later?
Sure that's fine. There's no documented restriction on allocating zero bytes and HeapAlloc
in practice does in fact return a non-null pointer if you ask for 0 bytes. Though tbh it's not something I'd be in the habit of doing.
Use non-null pointer for malloc(0) as mentioned in #3576 to detect leaks and double free of
malloc(0)
addresses.