UB with NtUnicodeStrMut::try_from_u16()

[smichaku]

The following program fails on miri with the following error:

use nt_string::unicode_string::NtUnicodeStrMut;

#[repr(C)]
struct UnicodeString {
    length: u16,
    maximum_length: u16,
    buffer: *mut u16,
}

fn overwrite(s: &mut NtUnicodeStrMut) {
    let len_in_elements = s.len() as usize / core::mem::size_of::<u16>();
    let x = u16::from(b'X');
    for i in 0..len_in_elements {
        s.as_mut_slice()[i] = x;
    }
}

fn main() {
    let mut buf: Vec<u16> = "Hello, world!".encode_utf16().collect();
    let mut us = NtUnicodeStrMut::try_from_u16(&mut buf).unwrap();
    println!("Before: {us}");
    overwrite(&mut us);
    println!("After:  {us}");
}

error: Undefined Behavior: trying to retag from <2677> for Unique permission at alloc892[0x0], but that t
ag only grants SharedReadOnly permission for this location
  --> C:\users\micha\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\nt-string-0.1.1\src\unicode_string\strmu
t.rs:36:18
   |
36 |         unsafe { slice::from_raw_parts_mut(self.raw.buffer, self.len_in_elements()) }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ thi
s error occurs as part of retag at alloc892[0x0..0x1a]
   |
   = 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: <2677> was created by a SharedReadOnly retag at offsets [0x0..0x1a]
  --> src\main.rs:20:18
   |
20 |     let mut us = NtUnicodeStrMut::try_from_u16(&mut buf).unwrap();
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: stack backtrace:
           0: nt_string::unicode_string::NtUnicodeStrMut::<'_>::as_mut_slice
               at C:\users\micha\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\nt-string-0.1.1\src\unicode_string\strmut
.rs:36:18: 36:84
           1: overwrite
               at src\main.rs:14:9: 14:25
           2: main
               at src\main.rs:22:5: 22:23

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error; 1 warning emitted

error: process didn't exit successfully: `C:\users\micha\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\bin\car
go-miri.exe runner target\miri\x86_64-pc-windows-msvc\debug\example.exe` (exit code: 1)

I think this is related to try_from_u16() transmuting an NtUnicodeStr to the result NtUnicodeStrMut instead of creating a new
RawNtString with a buffer field of *mut u16.

The following patch seems to fix it:

diff --git a/src/unicode_string/strmut.rs b/src/unicode_string/strmut.rs
index b54c9b3..f0245cc 100644
--- a/src/unicode_string/strmut.rs
+++ b/src/unicode_string/strmut.rs
@@ -122,14 +122,21 @@ impl<'a> NtUnicodeStrMut<'a> {
     ///
     /// [`try_from_u16_until_nul`]: Self::try_from_u16_until_nul
     pub fn try_from_u16(buffer: &mut [u16]) -> Result<Self> {
-        let unicode_str = NtUnicodeStr::try_from_u16(buffer)?;
-
-        // SAFETY: `unicode_str` was created from a mutable `buffer` and
-        // `NtUnicodeStr` and `NtUnicodeStrMut` have the same memory layout,
-        // so we can safely transmute `NtUnicodeStr` to `NtUnicodeStrMut`.
-        let unicode_str_mut = unsafe { mem::transmute(unicode_str) };
-
-        Ok(unicode_str_mut)
+        let elements = buffer.len();
+        let length_usize = elements
+            .checked_mul(mem::size_of::<u16>())
+            .ok_or(NtStringError::BufferSizeExceedsU16)?;
+        let length =
+            u16::try_from(length_usize).map_err(|_| NtStringError::BufferSizeExceedsU16)?;
+
+        Ok(Self {
+            raw: RawNtString {
+                length,
+                maximum_length: length,
+                buffer: buffer.as_mut_ptr(),
+            },
+            _lifetime: PhantomData,
+        })
     }
 
     /// Creates an [`NtUnicodeStrMut`] from an existing [`u16`] string buffer that contains at least one NUL character.

This is also relevant to try_from_u16_until_nul() and perhaps elsewhere.

[smichaku]

Opened PR: #2

[ColinFinck]

Fixed via #2