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

Potential Undefined Behavior in data_u16_to_gpu Due to Unsafe Pointer Usage #19

Open
lwz23 opened this issue Nov 30, 2024 · 1 comment

Comments

@lwz23
Copy link

lwz23 commented Nov 30, 2024

Description
The data_u16_to_gpu function contains unsafe code that can lead to undefined behavior (UB) due to unchecked pointer usage and assumptions about input validity. These issues undermine the safety guarantees of Rust and can lead to runtime crashes or memory corruption.

let data_slice: &[u16] = std::slice::from_raw_parts(data, nitems);

Code in Question:

pub fn data_u16_to_gpu(
    &self,
    data: *const u16,
    data_layout: Layout,
    nitems: usize,
    rows: i64,
    cols: i64,
    cols_capacity: i64,
) -> Result<OpenCLTensor, OpenCLError> {
    unsafe {
        let buf = Buffer::builder()
            .queue(self.queue.clone())
            .len(nitems)
            .build()?;
        let mut event = Event::empty();
        let data_slice: &[u16] = std::slice::from_raw_parts(data, nitems);
        buf.cmd()
            .write(data_slice)
            .block(false)
            .enew(&mut event)
            .enq()?;
        Ok(OpenCLTensor {
            buf,
            initial_write_event: Some(event),
            last_event: None,
            data,
            data_layout,
            nitems,
            rows,
            cols,
            cols_capacity,
            queue: self.queue.clone(),
            cl: self.clone(),
        })
    }
}

Problems:
this function is 'pub' so the user can control the 'data' field, if the 'data' is eg. a null pointer, there will be a UB

Steps to Reproduce:
Here is an example that might lead to UB:

let invalid_data = std::ptr::null(); // Null pointer
let nitems = 100;
let layout = Layout::from_size_align(100 * std::mem::size_of::<u16>(), 2).unwrap();

let result = gpu_context.data_u16_to_gpu(
    invalid_data,
    layout,
    nitems,
    10, // rows
    10, // cols
    10, // cols_capacity
);

assert!(result.is_err()); // Undefined behavior may occur before reaching this point

Suggestion

  1. mark this function as 'unsafe'.
  2. add 'assert' in the function body.

Expected Behavior:
Validate that data is non-null, properly aligned, and points to valid memory.

Additional Context:
This function operates in an unsafe block, and mistakes in pointer handling can lead to serious consequences, such as crashes, data corruption, or security vulnerabilities. Input validation and robust error handling are essential to ensure soundness in Rust.

@lwz23
Copy link
Author

lwz23 commented Dec 2, 2024

Unmatained?

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

1 participant