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

Maybe unsound in dma::write #117

Open
lwz23 opened this issue Dec 4, 2024 · 0 comments
Open

Maybe unsound in dma::write #117

lwz23 opened this issue Dec 4, 2024 · 0 comments

Comments

@lwz23
Copy link

lwz23 commented Dec 4, 2024

Hello, Thank you for your contribution to the project. I noticed the following code:

pub struct DmaArray<T> {
    pub ptr: NonNull<T>,
    pub length: usize,
    pub phys: usize,
    allocator: Arc<LockedHeap>,
    token: AtomicBool,
}

pub fn write(&mut self, index: usize, value: T) {
        assert!(!self.token_held(), "DmaArray accessed while token held!");
        assert!(index < self.length);
        unsafe {
            ptr::write(self.ptr.as_ptr().add(index), value);
        }
    }

I think this code might have an Unsound problem, but I'm not sure because it doesn't seem to work in my current environment, so I'll just try to simulate it.
This can be exploited by constructing a DmaArray instance with logical inconsistencies between its pointer (ptr), length (length), and the actual memory it points to.

Here is my PoC:

use std::ptr;
use std::ptr::NonNull;

pub struct DmaArray<T> {
    pub ptr: NonNull<T>,
    pub length: usize,
    pub phys: usize,
}

impl<T> DmaArray<T> {
    pub fn write(&mut self, index: usize, value: T) {
        assert!(index < self.length);
        unsafe {
            ptr::write(self.ptr.as_ptr().add(index), value);
        }
    }
}

fn main() {
    let mut vec = Vec::new();

    let dma_array = DmaArray {
        ptr: NonNull::new(vec.as_mut_ptr()).unwrap(),
        length: 1, 
        phys: 0,
    };

    let mut dma_array = dma_array;
    dma_array.write(0, 42u8);

    println!("Write completed");
}

Steps to Reproduce

  1. Create a Vec with zero elements.
  2. Obtain its raw pointer using vec.as_mut_ptr() and wrap it in NonNull.
  3. Construct a DmaArray instance with length > 0, even though the actual Vec capacity is 0.
  4. Invoke the write method, which accesses memory beyond the allocated bounds.

Sorry, my current environment doesn't seem to be able to run this project, so I can only simulate it in this way and delete some content. For security reasons, I raised this issue, please don't mind if this is a false positive.

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