HeaderMap error: Undefined Behavior: trying to retag from _ for Unique permission

[hjr3]

In https://github.com/hyperium/hyper/pull/3375 we ran into the following error:

error: Undefined Behavior: trying to retag from <1400070> for Unique permission at alloc405811[0x28], but that tag only grants SharedReadOnly permission for this location
    --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/http-0.2.10/src/header/map.rs:2111:35
     |
2111 |         let entry = unsafe { &mut (*self.map).entries[self.entry] };
     |                                   ^^^^^^^^^^^^^^^^^^^
     |                                   |
     |                                   trying to retag from <1400070> for Unique permission at alloc405811[0x28], but that tag only grants SharedReadOnly permission for this location
     |                                   this error occurs as part of retag at alloc405811[0x28..0x40]
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
     = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1400070> was created by a SharedReadOnly retag at offsets [0x0..0x60]
    --> src/proto/h1/role.rs:1524:26
     |

It was observed that this error is not occurring in v0.2.9.

I reproduced the error with this test in test/header_map.rs:

#[test]
fn repro_retag_error() {
    let mut headers = HeaderMap::new();
    headers.insert(
        HeaderName::from_static("chunky-trailer"),
        HeaderValue::from_static("header data"),
    );

    let _foo = &headers.iter().next();
}

I did a git bisect and identified the error:

$ git bisect start
$ git bisect good v0.2.9
$ git bisect bad v0.2.10
$ git bisect run sh -c 'RUSTFLAGS=-Adropping_copy_types MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test chunked_with_title_case_headers'

...

78e3d37563b0fe83886abffdcd8569117d45fee9 is the first bad commit
commit 78e3d37563b0fe83886abffdcd8569117d45fee9
Author: Discord9 <discord9@163.com>
Date:   Thu Jul 13 18:29:29 2023 +0800

    make miri happy

 src/header/map.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
bisect found first bad commit

The above change was introduced in https://github.com/hyperium/http/pull/616

Note: I had to specifyRUSTFLAGS=-Adropping_copy_types to avoid the following errors when building v0.2.9 on rustc 1.75.0-nightly (75b064d26 2023-11-01):

error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
    --> src/header/map.rs:1155:17
     |
1155 |                 drop(danger); // Make lint happy
     |                 ^^^^^------^
     |                      |
     |                      argument has type `bool`
     |
     = note: use `let _ = ...` to ignore the expression or result
note: the lint level is defined here
    --> src/lib.rs:161:9
     |
161  | #![deny(warnings, missing_docs, missing_debug_implementations)]
     |         ^^^^^^^^
     = note: `#[deny(dropping_copy_types)]` implied by `#[deny(warnings)]`

error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
    --> src/header/map.rs:1258:17
     |
1258 |                 drop(danger);
     |                 ^^^^^------^
     |                      |
     |                      argument has type `bool`
     |
     = note: use `let _ = ...` to ignore the expression or result

error: could not compile `http` (lib) due to 2 previous errors

[seanmonstar]

Weird. So a revert is good enough?

[hjr3]

I believe so. Playing with this more, miri added some additional information:

help: <122902> was created by a SharedReadOnly retag at offsets [0x0..0x60]
    --> /Code/http/src/header/map.rs:815:22
     |
815  |                 map: self as *const _ as *mut _,
     |                      ^^^^

Which is here: https://github.com/hyperium/http/blob/f5f31f0651774a5d68f2d5051b70e0bd0ccae577/src/header/map.rs#L815

I think the issue with 78e3d37563b0fe83886abffdcd8569117d45fee9 is:

• HeaderMap::iter converts &HeaderMap<T> to *mut HeaderMap<T>
• IterMut::next_unsafe uses that *mut HeaderMap<T> to pull out a&mut Bucket<T>

Thus, we have a &mut from something that was originally & and that is undefined behavior.

[seanmonstar]

Closes in #642