rusticl/query: fix use-after-free, but also fix incorrect usage of unsafe

`PipeQuery::new` create a `PipeQuery` wrapper before handling errors, so
we ended up calling `drop` and destroy_query, meaning the latter ended up
being called twice.

While at it, also restrict visibility of related methods and add some
unsafe declarations.

Fixes: 52e53938c3 ("rusticl: Wrap pipe queries")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24938>
This commit is contained in:
Karol Herbst 2023-08-29 20:20:24 +02:00 committed by Marge Bot
parent 6d3a7c1773
commit 8e4d51aa1f
2 changed files with 28 additions and 13 deletions

View file

@ -522,15 +522,21 @@ impl PipeContext {
}
}
pub fn create_query(&self, query_type: c_uint, index: c_uint) -> *mut pipe_query {
pub(crate) fn create_query(&self, query_type: c_uint, index: c_uint) -> *mut pipe_query {
unsafe { self.pipe.as_ref().create_query.unwrap()(self.pipe.as_ptr(), query_type, index) }
}
pub fn end_query(&self, pq: *mut pipe_query) -> bool {
/// # Safety
///
/// usual rules on raw mut pointers apply, specifically no concurrent access
pub(crate) unsafe fn end_query(&self, pq: *mut pipe_query) -> bool {
unsafe { self.pipe.as_ref().end_query.unwrap()(self.pipe.as_ptr(), pq) }
}
pub fn get_query_result(
/// # Safety
///
/// usual rules on raw mut pointers apply, specifically no concurrent access
pub(crate) unsafe fn get_query_result(
&self,
pq: *mut pipe_query,
wait: bool,
@ -539,7 +545,10 @@ impl PipeContext {
unsafe { self.pipe.as_ref().get_query_result.unwrap()(self.pipe.as_ptr(), pq, wait, pqr) }
}
pub fn destroy_query(&self, pq: *mut pipe_query) {
/// # Safety
///
/// usual rules on raw mut pointers apply, specifically no concurrent access
pub(crate) unsafe fn destroy_query(&self, pq: *mut pipe_query) {
unsafe { self.pipe.as_ref().destroy_query.unwrap()(self.pipe.as_ptr(), pq) }
}

View file

@ -45,22 +45,27 @@ impl<'a, R> PipeQuery<'a, R> {
if pq.is_null() {
return None;
}
let res = Some(Self {
// SAFETY: we are the only owner of that valid pointer
unsafe {
if !ctx.end_query(pq) {
ctx.destroy_query(pq);
return None;
}
}
Some(Self {
query: pq,
ctx: ctx,
_result_marker: Default::default(),
});
if !ctx.end_query(pq) {
ctx.destroy_query(pq);
return None;
}
res
})
}
}
impl<'a, R> Drop for PipeQuery<'a, R> {
fn drop(&mut self) {
self.ctx.destroy_query(self.query);
// SAFETY: we are the only owner of that valid pointer
unsafe {
self.ctx.destroy_query(self.query);
}
}
}
@ -78,7 +83,8 @@ impl QueryReadTrait for PipeQuery<'_, u64> {
fn read(&mut self, wait: bool) -> Option<u64> {
let mut raw_result = pipe_query_result::default();
if self.ctx.get_query_result(self.query, wait, &mut raw_result) {
// SAFETY: we guarentee unique access through our `&mut self` reference above.
if unsafe { self.ctx.get_query_result(self.query, wait, &mut raw_result) } {
// SAFETY: We know this is the right type
// because of the trait bound on PipeQueryGen binds the
// query type with the result type.