Return non-null pointer from malloc(0)

[tiif]

Use non-null pointer for malloc(0) as mentioned in #3576 to detect leaks and double free of malloc(0) addresses.

[tiif]

@rustbot ready

[RalfJung]

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.

[tiif]

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;
    }

[RalfJung]

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;.

[tiif]

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;
    }
}

[RalfJung]

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.

[tiif]

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!

[RalfJung]

@rustbot author

[rustbot]

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:

• 5e72719

[tiif]

@rustbot ready

[RalfJung]

This works, but please move it up above the comment that says we will round down.

Also, you can use Align::ONE to avoid an unwrap here.

[RalfJung]

Awesome. :) Please squash the commits.

[RalfJung]

@bors r+

[bors]

📌 Commit 3a2524a has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3a2524a with merge 6714e03...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 6714e03 to master...

[RalfJung]

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?

[ChrisDenton]

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.