From 6e62c690ba21c27fb336a2c8519a306df3161288 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Fri, 16 Sep 2022 16:10:44 +0200 Subject: [PATCH 1/5] Make nightly Clippy happy. --- src/array.rs | 2 +- src/slice_container.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/array.rs b/src/array.rs index f42b2e161..b1464f926 100644 --- a/src/array.rs +++ b/src/array.rs @@ -451,7 +451,7 @@ impl PyArray { where ID: IntoDimension, { - let flags = if is_fortran { 1 } else { 0 }; + let flags = c_int::from(is_fortran); Self::new_uninit(py, dims, ptr::null_mut(), flags) } diff --git a/src/slice_container.rs b/src/slice_container.rs index 3cbfc6110..6ae1f6d35 100644 --- a/src/slice_container.rs +++ b/src/slice_container.rs @@ -1,4 +1,4 @@ -use std::{mem, slice}; +use std::{mem, ptr}; use ndarray::{ArrayBase, Dimension, OwnedRepr}; use pyo3::pyclass; @@ -17,7 +17,7 @@ unsafe impl Send for PySliceContainer {} impl From> for PySliceContainer { fn from(data: Box<[T]>) -> Self { unsafe fn drop_boxed_slice(ptr: *mut u8, len: usize, _cap: usize) { - let _ = Box::from_raw(slice::from_raw_parts_mut(ptr as *mut T, len) as *mut [T]); + let _ = Box::from_raw(ptr::slice_from_raw_parts_mut(ptr as *mut T, len)); } // FIXME(adamreichold): Use `Box::into_raw` when From ff4699e772d863dd00057a74e040603223f4111b Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Fri, 16 Sep 2022 16:13:45 +0200 Subject: [PATCH 2/5] Use slice:as_mut_ptr instead of slice::as_ptr to enable modifications via the returned pointers. --- src/array.rs | 8 ++++---- src/convert.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/array.rs b/src/array.rs index b1464f926..bba97c7f3 100644 --- a/src/array.rs +++ b/src/array.rs @@ -676,9 +676,9 @@ impl PyArray { /// assert_eq!(pyarray.readonly().as_array(), array![[1, 2], [3, 4]]); /// }); /// ``` - pub fn from_owned_array<'py>(py: Python<'py>, arr: Array) -> &'py Self { + pub fn from_owned_array<'py>(py: Python<'py>, mut arr: Array) -> &'py Self { let (strides, dims) = (arr.npy_strides(), arr.raw_dim()); - let data_ptr = arr.as_ptr(); + let data_ptr = arr.as_mut_ptr(); unsafe { Self::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) } } @@ -1071,9 +1071,9 @@ impl PyArray { /// assert!(pyarray.readonly().as_array().get(0).unwrap().as_ref(py).is_instance_of::().unwrap()); /// }); /// ``` - pub fn from_owned_object_array<'py, T>(py: Python<'py>, arr: Array, D>) -> &'py Self { + pub fn from_owned_object_array<'py, T>(py: Python<'py>, mut arr: Array, D>) -> &'py Self { let (strides, dims) = (arr.npy_strides(), arr.raw_dim()); - let data_ptr = arr.as_ptr() as *const PyObject; + let data_ptr = arr.as_mut_ptr() as *const PyObject; unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) } } } diff --git a/src/convert.rs b/src/convert.rs index ded0cf8b2..2dd9a0076 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -48,10 +48,10 @@ impl IntoPyArray for Box<[T]> { type Item = T; type Dim = Ix1; - fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray { + fn into_pyarray<'py>(mut self, py: Python<'py>) -> &'py PyArray { let dims = [self.len()]; let strides = [mem::size_of::() as npy_intp]; - let data_ptr = self.as_ptr(); + let data_ptr = self.as_mut_ptr(); unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, self) } } } @@ -60,10 +60,10 @@ impl IntoPyArray for Vec { type Item = T; type Dim = Ix1; - fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray { + fn into_pyarray<'py>(mut self, py: Python<'py>) -> &'py PyArray { let dims = [self.len()]; let strides = [mem::size_of::() as npy_intp]; - let data_ptr = self.as_ptr(); + let data_ptr = self.as_mut_ptr(); unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, self) } } } From ba025b3300f302809b5a021b0e4a1c2b4e2fe16f Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Fri, 16 Sep 2022 16:31:32 +0200 Subject: [PATCH 3/5] Derive data pointer only after dissolving Box to avoid unsound aliasing. --- src/array.rs | 27 +++++++++++++++++++++------ src/convert.rs | 23 ++++++++++++++++++----- src/slice_container.rs | 16 ++++++++-------- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/array.rs b/src/array.rs index bba97c7f3..9415180ae 100644 --- a/src/array.rs +++ b/src/array.rs @@ -512,18 +512,17 @@ impl PyArray { Self::from_owned_ptr(py, ptr) } - pub(crate) unsafe fn from_raw_parts<'py, ID, C>( + pub(crate) unsafe fn from_raw_parts<'py, ID>( py: Python<'py>, dims: ID, strides: *const npy_intp, data_ptr: *const T, - container: C, + container: PySliceContainer, ) -> &'py Self where ID: IntoDimension, - PySliceContainer: From, { - let container = PyClassInitializer::from(PySliceContainer::from(container)) + let container = PyClassInitializer::from(container) .create_cell(py) .expect("Failed to create slice container"); @@ -679,7 +678,15 @@ impl PyArray { pub fn from_owned_array<'py>(py: Python<'py>, mut arr: Array) -> &'py Self { let (strides, dims) = (arr.npy_strides(), arr.raw_dim()); let data_ptr = arr.as_mut_ptr(); - unsafe { Self::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) } + unsafe { + Self::from_raw_parts( + py, + dims, + strides.as_ptr(), + data_ptr, + PySliceContainer::from(arr), + ) + } } /// Get a reference of the specified element if the given index is valid. @@ -1074,7 +1081,15 @@ impl PyArray { pub fn from_owned_object_array<'py, T>(py: Python<'py>, mut arr: Array, D>) -> &'py Self { let (strides, dims) = (arr.npy_strides(), arr.raw_dim()); let data_ptr = arr.as_mut_ptr() as *const PyObject; - unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) } + unsafe { + Self::from_raw_parts( + py, + dims, + strides.as_ptr(), + data_ptr, + PySliceContainer::from(arr), + ) + } } } diff --git a/src/convert.rs b/src/convert.rs index 2dd9a0076..e9c2c5dbb 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -10,6 +10,7 @@ use crate::dtype::Element; use crate::error::MAX_DIMENSIONALITY_ERR; use crate::npyffi::{self, npy_intp}; use crate::sealed::Sealed; +use crate::slice_container::PySliceContainer; /// Conversion trait from owning Rust types into [`PyArray`]. /// @@ -48,11 +49,15 @@ impl IntoPyArray for Box<[T]> { type Item = T; type Dim = Ix1; - fn into_pyarray<'py>(mut self, py: Python<'py>) -> &'py PyArray { - let dims = [self.len()]; + fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray { + let container = PySliceContainer::from(self); + let dims = [container.len]; let strides = [mem::size_of::() as npy_intp]; - let data_ptr = self.as_mut_ptr(); - unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, self) } + // The data pointer is derived only after dissolving `Box` into `PySliceContainer` + // to avoid unsound aliasing of Box<[T]> which is currently noalias, + // c.f. https://github.com/rust-lang/unsafe-code-guidelines/issues/326 + let data_ptr = container.ptr as *mut T; + unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, container) } } } @@ -64,7 +69,15 @@ impl IntoPyArray for Vec { let dims = [self.len()]; let strides = [mem::size_of::() as npy_intp]; let data_ptr = self.as_mut_ptr(); - unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, self) } + unsafe { + PyArray::from_raw_parts( + py, + dims, + strides.as_ptr(), + data_ptr, + PySliceContainer::from(self), + ) + } } } diff --git a/src/slice_container.rs b/src/slice_container.rs index 6ae1f6d35..ba57e2e12 100644 --- a/src/slice_container.rs +++ b/src/slice_container.rs @@ -6,8 +6,8 @@ use pyo3::pyclass; /// Utility type to safely store `Box<[_]>` or `Vec<_>` on the Python heap #[pyclass] pub(crate) struct PySliceContainer { - ptr: *mut u8, - len: usize, + pub(crate) ptr: *mut u8, + pub(crate) len: usize, cap: usize, drop: unsafe fn(*mut u8, usize, usize), } @@ -22,13 +22,13 @@ impl From> for PySliceContainer { // FIXME(adamreichold): Use `Box::into_raw` when // `*mut [T]::{as_mut_ptr, len}` become stable and compatible with our MSRV. - let ptr = data.as_ptr() as *mut u8; + let mut data = mem::ManuallyDrop::new(data); + + let ptr = data.as_mut_ptr() as *mut u8; let len = data.len(); let cap = 0; let drop = drop_boxed_slice::; - mem::forget(data); - Self { ptr, len, @@ -46,13 +46,13 @@ impl From> for PySliceContainer { // FIXME(adamreichold): Use `Vec::into_raw_parts` // when it becomes stable and compatible with our MSRV. - let ptr = data.as_ptr() as *mut u8; + let mut data = mem::ManuallyDrop::new(data); + + let ptr = data.as_mut_ptr() as *mut u8; let len = data.len(); let cap = data.capacity(); let drop = drop_vec::; - mem::forget(data); - Self { ptr, len, From 1eb9dfe32e72def510e3bac1a559a59c7e8ffa71 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Fri, 16 Sep 2022 16:34:43 +0200 Subject: [PATCH 4/5] Remove unnecessary generics from PyArray::from_raw_parts. --- src/array.rs | 9 +++------ src/convert.rs | 6 +++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/array.rs b/src/array.rs index 9415180ae..1128b4275 100644 --- a/src/array.rs +++ b/src/array.rs @@ -512,16 +512,13 @@ impl PyArray { Self::from_owned_ptr(py, ptr) } - pub(crate) unsafe fn from_raw_parts<'py, ID>( + pub(crate) unsafe fn from_raw_parts<'py>( py: Python<'py>, - dims: ID, + dims: D, strides: *const npy_intp, data_ptr: *const T, container: PySliceContainer, - ) -> &'py Self - where - ID: IntoDimension, - { + ) -> &'py Self { let container = PyClassInitializer::from(container) .create_cell(py) .expect("Failed to create slice container"); diff --git a/src/convert.rs b/src/convert.rs index e9c2c5dbb..10d19693a 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -2,7 +2,7 @@ use std::{mem, os::raw::c_int, ptr}; -use ndarray::{ArrayBase, Data, Dimension, IntoDimension, Ix1, OwnedRepr}; +use ndarray::{ArrayBase, Data, Dim, Dimension, IntoDimension, Ix1, OwnedRepr}; use pyo3::Python; use crate::array::PyArray; @@ -51,7 +51,7 @@ impl IntoPyArray for Box<[T]> { fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray { let container = PySliceContainer::from(self); - let dims = [container.len]; + let dims = Dim([container.len]); let strides = [mem::size_of::() as npy_intp]; // The data pointer is derived only after dissolving `Box` into `PySliceContainer` // to avoid unsound aliasing of Box<[T]> which is currently noalias, @@ -66,7 +66,7 @@ impl IntoPyArray for Vec { type Dim = Ix1; fn into_pyarray<'py>(mut self, py: Python<'py>) -> &'py PyArray { - let dims = [self.len()]; + let dims = Dim([self.len()]); let strides = [mem::size_of::() as npy_intp]; let data_ptr = self.as_mut_ptr(); unsafe { From b1c1e4f84ed1d76121fd411ad7855512b5733867 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Fri, 16 Sep 2022 16:36:16 +0200 Subject: [PATCH 5/5] Update change log and bump crate version. --- CHANGELOG.md | 3 +++ Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaed3f2a9..5d73e940a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ - Unreleased +- v0.17.2 + - Fix unsound aliasing into `Box<[T]>` when converting them into NumPy arrays. ([#351](https://github.com/PyO3/rust-numpy/pull/351)) + - v0.17.1 - Fix use-after-free in `PyArray::resize`, `PyArray::reshape` and `PyArray::reshape_with_order`. ([#341](https://github.com/PyO3/rust-numpy/pull/341)) - Fix UB in `ToNpyDims::as_dims_ptr` with dimensions of dynamic size (-1). ([#344](https://github.com/PyO3/rust-numpy/pull/344)) diff --git a/Cargo.toml b/Cargo.toml index 21e5d3cfa..9ae5c3e22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "numpy" -version = "0.17.1" +version = "0.17.2" authors = [ "The rust-numpy Project Developers", "PyO3 Project and Contributors "