Skip to content

Aliasing UB in Convert for &mut T and Buf (Miri Stacked/Tree borrow violations) #1311

Description

@zhouxt1

Hi, I have found the following aliasing bugs in git2.

Convert for &mut T in src/call.rs

The first problem is as follows:

    impl<T> Convert<*mut T> for &mut T {
        fn convert(&self) -> *mut T {
            &**self as *const T as *mut T
        }
    }

The problem is it first created a shared ref &**self, then cast it to a mutable raw pointer. So mutating through the raw pointer is UB.

Fix:

impl<T> Convert<*mut T> for &mut T {
    fn convert(&self) -> *mut T {
        unsafe { ptr::read(self) as *mut T }
    }
}

copy the &mut T out of *self instead of reborrowing so the pointer keeps write permission.

raw in src/buf.rs

The problem is in:

pub struct Buf {
    raw: raw::git_buf,
}

impl Binding for Buf {
    type Raw = *mut raw::git_buf;
    unsafe fn from_raw(raw: *mut raw::git_buf) -> Buf {
        Buf { raw: *raw }
    }
    fn raw(&self) -> *mut raw::git_buf {
        &self.raw as *const _ as *mut _
    }
}

It is a similar problem, casting a shared reference to a mutable pointer. So mutating it results in a UB.

Fix: use a cell which allows interior mutability:

pub struct Buf {
    raw: UnsafeCell<raw::git_buf>,
}

also, update the corresponding sites that uses raw.

I have run confirmed the bug running it using the MiriLLI tools (with tests that calls these from libgit2). The two are UB under both tree borrow and stacked borrow models.

I am happy to open a PR to fix these bugs. Thanks

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions