rusticl: fix UB in CLProp machinery

Viewing structs as a collection of u8 is not generally sound. Any padding bytes might be
uninitialized and creating an integer from uninitialized memory constitutes producing an invalid
value, which is instant UB.

Since we only copy these bytes around, the fix is to simply work with MaybeUninit<u8>, which can handle uninitialized memory just fine, instead.

See: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23652>
This commit is contained in:
LingMan 2023-06-14 09:02:53 +02:00 committed by Marge Bot
parent fdcb86168d
commit 0535948535
9 changed files with 62 additions and 45 deletions

View file

@ -13,12 +13,13 @@ use rusticl_proc_macros::cl_info_entrypoint;
use std::collections::HashSet;
use std::iter::FromIterator;
use std::mem::MaybeUninit;
use std::slice;
use std::sync::Arc;
#[cl_info_entrypoint(cl_get_context_info)]
impl CLInfo<cl_context_info> for cl_context {
fn query(&self, q: cl_context_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_context_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let ctx = self.get_ref()?;
Ok(match q {
CL_CONTEXT_DEVICES => {

View file

@ -13,7 +13,7 @@ use rusticl_proc_macros::cl_info_entrypoint;
use std::cmp::min;
use std::ffi::CStr;
use std::mem::size_of;
use std::mem::{size_of, MaybeUninit};
use std::ptr;
use std::sync::Arc;
@ -29,7 +29,7 @@ type ClDevIdpAccelProps = cl_device_integer_dot_product_acceleration_properties_
#[cl_info_entrypoint(cl_get_device_info)]
impl CLInfo<cl_device_info> for cl_device_id {
fn query(&self, q: cl_device_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_device_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let dev = self.get_ref()?;
// curses you CL_DEVICE_INTEGER_DOT_PRODUCT_ACCELERATION_PROPERTIES_4x8BIT_PACKED_KHR

View file

@ -9,12 +9,13 @@ use rusticl_proc_macros::cl_entrypoint;
use rusticl_proc_macros::cl_info_entrypoint;
use std::collections::HashSet;
use std::mem::MaybeUninit;
use std::ptr;
use std::sync::Arc;
#[cl_info_entrypoint(cl_get_event_info)]
impl CLInfo<cl_event_info> for cl_event {
fn query(&self, q: cl_event_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_event_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let event = self.get_ref()?;
Ok(match *q {
CL_EVENT_COMMAND_EXECUTION_STATUS => cl_prop::<cl_int>(event.status()),
@ -40,7 +41,7 @@ impl CLInfo<cl_event_info> for cl_event {
#[cl_info_entrypoint(cl_get_event_profiling_info)]
impl CLInfo<cl_profiling_info> for cl_event {
fn query(&self, q: cl_profiling_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_profiling_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let event = self.get_ref()?;
if event.cmd_type == CL_COMMAND_USER {
// CL_PROFILING_INFO_NOT_AVAILABLE [...] if event is a user event object.

View file

@ -10,7 +10,7 @@ use rusticl_opencl_gen::*;
use rusticl_proc_macros::cl_entrypoint;
use rusticl_proc_macros::cl_info_entrypoint;
use std::mem;
use std::mem::{self, MaybeUninit};
use std::os::raw::c_void;
use std::ptr;
use std::slice;
@ -18,7 +18,7 @@ use std::sync::Arc;
#[cl_info_entrypoint(cl_get_kernel_info)]
impl CLInfo<cl_kernel_info> for cl_kernel {
fn query(&self, q: cl_kernel_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_kernel_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let kernel = self.get_ref()?;
Ok(match q {
CL_KERNEL_ATTRIBUTES => cl_prop::<&str>(&kernel.attributes_string),
@ -41,7 +41,7 @@ impl CLInfo<cl_kernel_info> for cl_kernel {
#[cl_info_entrypoint(cl_get_kernel_arg_info)]
impl CLInfoObj<cl_kernel_arg_info, cl_uint> for cl_kernel {
fn query(&self, idx: cl_uint, q: cl_kernel_arg_info) -> CLResult<Vec<u8>> {
fn query(&self, idx: cl_uint, q: cl_kernel_arg_info) -> CLResult<Vec<MaybeUninit<u8>>> {
let kernel = self.get_ref()?;
// CL_INVALID_ARG_INDEX if arg_index is not a valid argument index.
@ -69,7 +69,11 @@ impl CLInfoObj<cl_kernel_arg_info, cl_uint> for cl_kernel {
#[cl_info_entrypoint(cl_get_kernel_work_group_info)]
impl CLInfoObj<cl_kernel_work_group_info, cl_device_id> for cl_kernel {
fn query(&self, dev: cl_device_id, q: cl_kernel_work_group_info) -> CLResult<Vec<u8>> {
fn query(
&self,
dev: cl_device_id,
q: cl_kernel_work_group_info,
) -> CLResult<Vec<MaybeUninit<u8>>> {
let kernel = self.get_ref()?;
// CL_INVALID_DEVICE [..] if device is NULL but there is more than one device associated with kernel.
@ -107,7 +111,7 @@ impl CLInfoObj<cl_kernel_sub_group_info, (cl_device_id, usize, *const c_void)> f
&self,
(d, _input_value_size, _input_value): (cl_device_id, usize, *const c_void),
_q: cl_program_build_info,
) -> CLResult<Vec<u8>> {
) -> CLResult<Vec<MaybeUninit<u8>>> {
let _kernel = self.get_ref()?;
let _dev = d.get_arc()?;

View file

@ -19,7 +19,7 @@ use rusticl_proc_macros::cl_info_entrypoint;
use std::alloc;
use std::alloc::Layout;
use std::cmp::Ordering;
use std::mem;
use std::mem::{self, MaybeUninit};
use std::os::raw::c_void;
use std::ptr;
use std::slice;
@ -211,7 +211,7 @@ fn validate_matching_buffer_flags(mem: &Mem, flags: cl_mem_flags) -> CLResult<()
#[cl_info_entrypoint(cl_get_mem_object_info)]
impl CLInfo<cl_mem_info> for cl_mem {
fn query(&self, q: cl_mem_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_mem_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let mem = self.get_ref()?;
Ok(match *q {
CL_MEM_ASSOCIATED_MEMOBJECT => {
@ -704,7 +704,7 @@ fn validate_buffer(
#[cl_info_entrypoint(cl_get_image_info)]
impl CLInfo<cl_image_info> for cl_mem {
fn query(&self, q: cl_image_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_image_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let mem = self.get_ref()?;
Ok(match *q {
CL_IMAGE_ARRAY_SIZE => cl_prop::<usize>(mem.image_desc.image_array_size),
@ -899,7 +899,7 @@ fn get_supported_image_formats(
#[cl_info_entrypoint(cl_get_sampler_info)]
impl CLInfo<cl_sampler_info> for cl_sampler {
fn query(&self, q: cl_sampler_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_sampler_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let sampler = self.get_ref()?;
Ok(match q {
CL_SAMPLER_ADDRESSING_MODE => cl_prop::<cl_addressing_mode>(sampler.addressing_mode),
@ -2263,7 +2263,7 @@ fn enqueue_migrate_mem_objects(
#[cl_info_entrypoint(cl_get_pipe_info)]
impl CLInfo<cl_pipe_info> for cl_mem {
fn query(&self, _q: cl_pipe_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, _q: cl_pipe_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
// CL_INVALID_MEM_OBJECT if pipe is a not a valid pipe object.
Err(CL_INVALID_MEM_OBJECT)
}

View file

@ -8,9 +8,11 @@ use rusticl_opencl_gen::*;
use rusticl_proc_macros::cl_entrypoint;
use rusticl_proc_macros::cl_info_entrypoint;
use std::mem::MaybeUninit;
#[cl_info_entrypoint(cl_get_platform_info)]
impl CLInfo<cl_platform_info> for cl_platform_id {
fn query(&self, q: cl_platform_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_platform_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
self.get_ref()?;
Ok(match q {
// TODO spirv

View file

@ -14,6 +14,7 @@ use rusticl_proc_macros::cl_info_entrypoint;
use std::ffi::CStr;
use std::ffi::CString;
use std::iter;
use std::mem::MaybeUninit;
use std::num::NonZeroUsize;
use std::os::raw::c_char;
use std::ptr;
@ -22,7 +23,7 @@ use std::sync::Arc;
#[cl_info_entrypoint(cl_get_program_info)]
impl CLInfo<cl_program_info> for cl_program {
fn query(&self, q: cl_program_info, vals: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_program_info, vals: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let prog = self.get_ref()?;
Ok(match q {
CL_PROGRAM_BINARIES => cl_prop::<Vec<*mut u8>>(prog.binaries(vals)),
@ -45,7 +46,7 @@ impl CLInfo<cl_program_info> for cl_program {
)
}
CL_PROGRAM_IL => match &prog.src {
ProgramSourceType::Il(il) => il.to_bin().to_vec(),
ProgramSourceType::Il(il) => to_maybeuninit_vec(il.to_bin().to_vec()),
_ => Vec::new(),
},
CL_PROGRAM_KERNEL_NAMES => cl_prop::<&str>(&*prog.kernels().join(";")),
@ -66,7 +67,7 @@ impl CLInfo<cl_program_info> for cl_program {
#[cl_info_entrypoint(cl_get_program_build_info)]
impl CLInfoObj<cl_program_build_info, cl_device_id> for cl_program {
fn query(&self, d: cl_device_id, q: cl_program_build_info) -> CLResult<Vec<u8>> {
fn query(&self, d: cl_device_id, q: cl_program_build_info) -> CLResult<Vec<MaybeUninit<u8>>> {
let prog = self.get_ref()?;
let dev = d.get_arc()?;
Ok(match q {

View file

@ -9,12 +9,13 @@ use rusticl_opencl_gen::*;
use rusticl_proc_macros::cl_entrypoint;
use rusticl_proc_macros::cl_info_entrypoint;
use std::mem::MaybeUninit;
use std::ptr;
use std::sync::Arc;
#[cl_info_entrypoint(cl_get_command_queue_info)]
impl CLInfo<cl_command_queue_info> for cl_command_queue {
fn query(&self, q: cl_command_queue_info, _: &[u8]) -> CLResult<Vec<u8>> {
fn query(&self, q: cl_command_queue_info, _: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>> {
let queue = self.get_ref()?;
Ok(match q {
CL_QUEUE_CONTEXT => {

View file

@ -11,14 +11,14 @@ use std::cmp;
use std::convert::TryInto;
use std::ffi::CStr;
use std::ffi::CString;
use std::mem::size_of;
use std::mem::{size_of, MaybeUninit};
use std::ops::BitAnd;
use std::os::raw::c_void;
use std::slice;
use std::sync::Arc;
pub trait CLInfo<I> {
fn query(&self, q: I, vals: &[u8]) -> CLResult<Vec<u8>>;
fn query(&self, q: I, vals: &[u8]) -> CLResult<Vec<MaybeUninit<u8>>>;
fn get_info(
&self,
@ -57,7 +57,7 @@ pub trait CLInfo<I> {
}
pub trait CLInfoObj<I, O> {
fn query(&self, o: O, q: I) -> CLResult<Vec<u8>>;
fn query(&self, o: O, q: I) -> CLResult<Vec<MaybeUninit<u8>>>;
fn get_info_obj(
&self,
@ -91,13 +91,13 @@ pub trait CLInfoObj<I, O> {
}
pub trait CLProp {
fn cl_vec(&self) -> Vec<u8>;
fn cl_vec(&self) -> Vec<MaybeUninit<u8>>;
}
macro_rules! cl_prop_for_type {
($ty: ty) => {
impl CLProp for $ty {
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
unsafe { slice::from_raw_parts((self as *const Self).cast(), size_of::<Self>()) }
.to_vec()
}
@ -120,22 +120,24 @@ cl_prop_for_type!(cl_image_format);
cl_prop_for_type!(cl_name_version);
impl CLProp for bool {
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
cl_prop::<cl_bool>(if *self { CL_TRUE } else { CL_FALSE })
}
}
impl CLProp for &str {
fn cl_vec(&self) -> Vec<u8> {
CString::new(*self)
.unwrap_or_default()
.into_bytes_with_nul()
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
to_maybeuninit_vec(
CString::new(*self)
.unwrap_or_default()
.into_bytes_with_nul(),
)
}
}
impl CLProp for &CStr {
fn cl_vec(&self) -> Vec<u8> {
self.to_bytes_with_nul().to_vec()
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
to_maybeuninit_vec(self.to_bytes_with_nul().to_vec())
}
}
@ -143,8 +145,8 @@ impl<T> CLProp for Vec<T>
where
T: CLProp,
{
fn cl_vec(&self) -> Vec<u8> {
let mut res: Vec<u8> = Vec::new();
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
let mut res: Vec<MaybeUninit<u8>> = Vec::new();
for i in self {
res.append(&mut i.cl_vec())
}
@ -156,7 +158,7 @@ impl<T> CLProp for &T
where
T: CLProp,
{
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
T::cl_vec(self)
}
}
@ -165,8 +167,8 @@ impl<T> CLProp for [T]
where
T: CLProp,
{
fn cl_vec(&self) -> Vec<u8> {
let mut res: Vec<u8> = Vec::new();
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
let mut res: Vec<MaybeUninit<u8>> = Vec::new();
for i in self {
res.append(&mut i.cl_vec())
}
@ -178,8 +180,8 @@ impl<T, const I: usize> CLProp for [T; I]
where
T: CLProp,
{
fn cl_vec(&self) -> Vec<u8> {
let mut res: Vec<u8> = Vec::new();
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
let mut res: Vec<MaybeUninit<u8>> = Vec::new();
for i in self {
res.append(&mut i.cl_vec())
}
@ -188,13 +190,13 @@ where
}
impl<T> CLProp for *const T {
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
(*self as usize).cl_vec()
}
}
impl<T> CLProp for *mut T {
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
(*self as usize).cl_vec()
}
}
@ -203,8 +205,8 @@ impl<T> CLProp for Properties<T>
where
T: CLProp + Default,
{
fn cl_vec(&self) -> Vec<u8> {
let mut res: Vec<u8> = Vec::new();
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
let mut res: Vec<MaybeUninit<u8>> = Vec::new();
for (k, v) in &self.props {
res.append(&mut k.cl_vec());
res.append(&mut v.cl_vec());
@ -218,12 +220,12 @@ impl<T> CLProp for Option<T>
where
T: CLProp,
{
fn cl_vec(&self) -> Vec<u8> {
fn cl_vec(&self) -> Vec<MaybeUninit<u8>> {
self.as_ref().map_or(Vec::new(), |v| v.cl_vec())
}
}
pub fn cl_prop<T: CLProp>(v: T) -> Vec<u8>
pub fn cl_prop<T: CLProp>(v: T) -> Vec<MaybeUninit<u8>>
where
T: Sized,
{
@ -297,6 +299,11 @@ pub fn event_list_from_cl(
Ok(res)
}
pub fn to_maybeuninit_vec<T: Copy>(v: Vec<T>) -> Vec<MaybeUninit<T>> {
// In my tests the compiler was smart enough to turn this into a noop
v.into_iter().map(MaybeUninit::new).collect()
}
pub fn check_cb<T>(cb: &Option<T>, user_data: *mut c_void) -> CLResult<()> {
// CL_INVALID_VALUE if pfn_notify is NULL but user_data is not NULL.
if cb.is_none() && !user_data.is_null() {