Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unsound aliasing into Box<[T]> when converting them into NumPy arrays #351

Merged
merged 5 commits into from
Sep 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/PyO3>"
Expand Down
44 changes: 28 additions & 16 deletions src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ impl<T: Element, D: Dimension> PyArray<T, D> {
where
ID: IntoDimension<Dim = D>,
{
let flags = if is_fortran { 1 } else { 0 };
let flags = c_int::from(is_fortran);
Self::new_uninit(py, dims, ptr::null_mut(), flags)
}

Expand Down Expand Up @@ -512,18 +512,14 @@ impl<T: Element, D: Dimension> PyArray<T, D> {
Self::from_owned_ptr(py, ptr)
}

pub(crate) unsafe fn from_raw_parts<'py, ID, C>(
pub(crate) unsafe fn from_raw_parts<'py>(
py: Python<'py>,
dims: ID,
dims: D,
strides: *const npy_intp,
data_ptr: *const T,
container: C,
) -> &'py Self
where
ID: IntoDimension<Dim = D>,
PySliceContainer: From<C>,
{
let container = PyClassInitializer::from(PySliceContainer::from(container))
container: PySliceContainer,
) -> &'py Self {
let container = PyClassInitializer::from(container)
.create_cell(py)
.expect("Failed to create slice container");

Expand Down Expand Up @@ -676,10 +672,18 @@ impl<T: Element, D: Dimension> PyArray<T, D> {
/// assert_eq!(pyarray.readonly().as_array(), array![[1, 2], [3, 4]]);
/// });
/// ```
pub fn from_owned_array<'py>(py: Python<'py>, arr: Array<T, D>) -> &'py Self {
pub fn from_owned_array<'py>(py: Python<'py>, mut arr: Array<T, D>) -> &'py Self {
let (strides, dims) = (arr.npy_strides(), arr.raw_dim());
let data_ptr = arr.as_ptr();
unsafe { Self::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) }
let data_ptr = arr.as_mut_ptr();
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.
Expand Down Expand Up @@ -1071,10 +1075,18 @@ impl<D: Dimension> PyArray<PyObject, D> {
/// assert!(pyarray.readonly().as_array().get(0).unwrap().as_ref(py).is_instance_of::<CustomElement>().unwrap());
/// });
/// ```
pub fn from_owned_object_array<'py, T>(py: Python<'py>, arr: Array<Py<T>, D>) -> &'py Self {
pub fn from_owned_object_array<'py, T>(py: Python<'py>, mut arr: Array<Py<T>, D>) -> &'py Self {
let (strides, dims) = (arr.npy_strides(), arr.raw_dim());
let data_ptr = arr.as_ptr() as *const PyObject;
unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, arr) }
let data_ptr = arr.as_mut_ptr() as *const PyObject;
unsafe {
Self::from_raw_parts(
py,
dims,
strides.as_ptr(),
data_ptr,
PySliceContainer::from(arr),
)
}
}
}

Expand Down
29 changes: 21 additions & 8 deletions src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

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;
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`].
///
Expand Down Expand Up @@ -49,22 +50,34 @@ impl<T: Element> IntoPyArray for Box<[T]> {
type Dim = Ix1;

fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray<Self::Item, Self::Dim> {
let dims = [self.len()];
let container = PySliceContainer::from(self);
let dims = Dim([container.len]);
let strides = [mem::size_of::<T>() as npy_intp];
let data_ptr = self.as_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) }
}
}

impl<T: Element> IntoPyArray for Vec<T> {
type Item = T;
type Dim = Ix1;

fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray<Self::Item, Self::Dim> {
let dims = [self.len()];
fn into_pyarray<'py>(mut self, py: Python<'py>) -> &'py PyArray<Self::Item, Self::Dim> {
let dims = Dim([self.len()]);
let strides = [mem::size_of::<T>() as npy_intp];
let data_ptr = self.as_ptr();
unsafe { PyArray::from_raw_parts(py, dims, strides.as_ptr(), data_ptr, self) }
let data_ptr = self.as_mut_ptr();
unsafe {
PyArray::from_raw_parts(
py,
dims,
strides.as_ptr(),
data_ptr,
PySliceContainer::from(self),
)
}
}
}

Expand Down
20 changes: 10 additions & 10 deletions src/slice_container.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use std::{mem, slice};
use std::{mem, ptr};

use ndarray::{ArrayBase, Dimension, OwnedRepr};
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),
}
Expand All @@ -17,18 +17,18 @@ unsafe impl Send for PySliceContainer {}
impl<T: Send> From<Box<[T]>> for PySliceContainer {
fn from(data: Box<[T]>) -> Self {
unsafe fn drop_boxed_slice<T>(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
// `*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::<T>;

mem::forget(data);

Self {
ptr,
len,
Expand All @@ -46,13 +46,13 @@ impl<T: Send> From<Vec<T>> 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::<T>;

mem::forget(data);

Self {
ptr,
len,
Expand Down