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

Uninitialized memory passed to safe functions in a few locations #26

Open
ammaraskar opened this issue Feb 5, 2021 · 1 comment
Open

Comments

@ammaraskar
Copy link

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that in these three code locations, unsafe memory is passed to Copy::copy, Copy::copy_mat and Gemv::gemv:

fn into(self) -> Vec<T> {
let n = self.len() as usize;
let mut x = Vec::with_capacity(n);
unsafe { x.set_len(n); }
Copy::copy(self, &mut x);
x
}

rust-blas/src/math/mat.rs

Lines 121 to 123 in bf6d331

unsafe { result.data.set_len(len); }
Copy::copy_mat(a, &mut result);

let mut result = Vec::with_capacity(n);
unsafe { result.set_len(n); }
let scale = Default::one();
let clear = Default::zero();
let t = Transpose::NoTrans;
Gemv::gemv(t, &scale, self, x, &clear, &mut result);

Copy and Gemv are currently public and safe traits to implement, maybe they should be marked as unsafe or the individual methods that can receive undefined memory marked as unsafe.

Also note that the Into<Vec<T>> implementation can invoke undefined behavior, see https://doc.rust-lang.org/std/mem/union.MaybeUninit.html#initialization-invariant

As a consequence, zero-initializing a variable of reference type causes instantaneous undefined behavior, no matter whether that reference ever gets used to access memory

which means that just doing something like this can invoke undefined behavior:

#![forbid(unsafe_code)]

use rblas::matrix::Matrix;
use rblas::vector::ops::Copy;
use rblas::vector::Vector;

#[derive(Clone, Debug)]
struct MyType(Box<u64>);

impl Copy for MyType {
    fn copy<V, W>(src: &V, dst: &mut W)
    where
        V: ?Sized + Vector<Self>,
        W: ?Sized + Vector<Self>,
    {
    }

    fn copy_mat(src: &Matrix<Self>, dst: &mut Matrix<Self>) {}
}

fn main() {
    let vec = vec![MyType(Box::new(42))];
    let as_blas_vector = &vec as &Vector<_>;

    let back_to_vec: Vec<MyType> = as_blas_vector.into();
}
@mikkyang
Copy link
Owner

Thanks for bringing this up. I'll look into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants