From fb043479304ec1b428deeab4554e2329f7073207 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Sun, 12 Jan 2025 13:44:31 -0800 Subject: [PATCH] examples/export: Replace unsound `to_padded_byte_vector()` implementation with `bytemuck`. The function `to_padded_byte_vector()` is unsound because: * It accepts an arbitrary `Vec` without checking that `T` contains no padding, which is UB to read in any way including by reinterpreting as `u8`s. * It produces a `Vec` which thinks it has a different alignment than the allocation was actually created with. To fix these problems, this change: * Uses `bytemuck` to check the no-padding condition. * Creates a new `Vec` instead of trying to reuse the existing one. (Conditional reuse would be possible, but more complex.) An alternative to `bytemuck` would be to make `to_padded_byte_vector()` an `unsafe fn` (or to accept `Vertex` only instead of `T`). However, I think it is valuable to demonstrate how to do this conversion using safe tools, to encourage people to use safe code instead of writing unsafe code without fully understanding the requirements. --- Cargo.toml | 1 + examples/export/main.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cee56ff5..b3e0f7a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ members = ["gltf-derive", "gltf-json"] [dev-dependencies] approx = "0.5" +bytemuck = { version = "1.21.0", features = ["derive"] } [dependencies] base64 = { optional = true, version = "0.13" } diff --git a/examples/export/main.rs b/examples/export/main.rs index 98332c69..4c65b766 100644 --- a/examples/export/main.rs +++ b/examples/export/main.rs @@ -16,7 +16,7 @@ enum Output { Binary, } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, bytemuck::NoUninit)] #[repr(C)] struct Vertex { position: [f32; 3], @@ -42,15 +42,14 @@ fn align_to_multiple_of_four(n: &mut usize) { *n = (*n + 3) & !3; } -fn to_padded_byte_vector(vec: Vec) -> Vec { - let byte_length = vec.len() * mem::size_of::(); - let byte_capacity = vec.capacity() * mem::size_of::(); - let alloc = vec.into_boxed_slice(); - let ptr = Box::<[T]>::into_raw(alloc) as *mut u8; - let mut new_vec = unsafe { Vec::from_raw_parts(ptr, byte_length, byte_capacity) }; +fn to_padded_byte_vector(data: &[T]) -> Vec { + let byte_slice: &[u8] = bytemuck::cast_slice(data); + let mut new_vec: Vec = byte_slice.to_owned(); + while new_vec.len() % 4 != 0 { new_vec.push(0); // pad to multiple of four bytes } + new_vec } @@ -171,7 +170,7 @@ fn export(output: Output) { let writer = fs::File::create("triangle/triangle.gltf").expect("I/O error"); json::serialize::to_writer_pretty(writer, &root).expect("Serialization error"); - let bin = to_padded_byte_vector(triangle_vertices); + let bin = to_padded_byte_vector(&triangle_vertices); let mut writer = fs::File::create("triangle/buffer0.bin").expect("I/O error"); writer.write_all(&bin).expect("I/O error"); } @@ -188,7 +187,7 @@ fn export(output: Output) { .try_into() .expect("file size exceeds binary glTF limit"), }, - bin: Some(Cow::Owned(to_padded_byte_vector(triangle_vertices))), + bin: Some(Cow::Owned(to_padded_byte_vector(&triangle_vertices))), json: Cow::Owned(json_string.into_bytes()), }; let writer = std::fs::File::create("triangle.glb").expect("I/O error");