From bfecaf404084c061d806fc28b622243d3afaedcf Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Sun, 19 Jan 2025 10:20:46 +0100 Subject: [PATCH] rusticl/kernel: rework validation in clSetKernelExecInfo We should use the cl_slice code to get proper validation, which also makes it simpler to read out data and gets rid of some UB there. This also fixes CL_KERNEL_EXEC_INFO_SVM_PTRS with param_value being null. Cc: mesa-stable Reviewed-by: Adam Jackson Part-of: (cherry picked from commit 35a9829391390a66d5163bae814ed28947ecbcee) --- .pick_status.json | 2 +- src/gallium/frontends/rusticl/api/kernel.rs | 25 +++++++++++++-------- src/gallium/frontends/rusticl/api/util.rs | 17 ++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 3ff8dfe7113..70e979061e3 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -2434,7 +2434,7 @@ "description": "rusticl/kernel: rework validation in clSetKernelExecInfo", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null, "notes": null diff --git a/src/gallium/frontends/rusticl/api/kernel.rs b/src/gallium/frontends/rusticl/api/kernel.rs index ae864a67fdc..629d8b2d637 100644 --- a/src/gallium/frontends/rusticl/api/kernel.rs +++ b/src/gallium/frontends/rusticl/api/kernel.rs @@ -501,22 +501,29 @@ fn set_kernel_exec_info( return Err(CL_INVALID_OPERATION); } - // CL_INVALID_VALUE ... if param_value is NULL - if param_value.is_null() { - return Err(CL_INVALID_VALUE); - } - // CL_INVALID_VALUE ... if the size specified by param_value_size is not valid. match param_name { CL_KERNEL_EXEC_INFO_SVM_PTRS | CL_KERNEL_EXEC_INFO_SVM_PTRS_ARM => { - // it's a list of pointers - if param_value_size % mem::size_of::<*const c_void>() != 0 { - return Err(CL_INVALID_VALUE); + // To specify that no SVM allocations will be accessed by a kernel other than those set + // as kernel arguments, specify an empty set by passing param_value_size equal to zero + // and param_value equal to NULL. + if !param_value.is_null() || param_value_size != 0 { + let _ = unsafe { + cl_slice::from_raw_parts_bytes_len::<*const c_void>( + param_value, + param_value_size, + )? + }; } } CL_KERNEL_EXEC_INFO_SVM_FINE_GRAIN_SYSTEM | CL_KERNEL_EXEC_INFO_SVM_FINE_GRAIN_SYSTEM_ARM => { - if param_value_size != mem::size_of::() { + let val = unsafe { + cl_slice::from_raw_parts_bytes_len::(param_value, param_value_size)? + }; + + // we must explicitly check that we only got one element + if val.len() != 1 { return Err(CL_INVALID_VALUE); } } diff --git a/src/gallium/frontends/rusticl/api/util.rs b/src/gallium/frontends/rusticl/api/util.rs index dcd059d8699..2222dd5d4df 100644 --- a/src/gallium/frontends/rusticl/api/util.rs +++ b/src/gallium/frontends/rusticl/api/util.rs @@ -560,6 +560,7 @@ pub mod cl_slice { use crate::api::util::CLResult; use mesa_rust_util::ptr::addr; use rusticl_opencl_gen::CL_INVALID_VALUE; + use std::ffi::c_void; use std::mem; use std::slice; @@ -586,6 +587,22 @@ pub mod cl_slice { unsafe { Ok(slice::from_raw_parts(data, len)) } } + /// same as [self::from_raw_parts] just that `len` is provided in bytes and must be a multiple + /// of Ts size. + #[inline] + pub unsafe fn from_raw_parts_bytes_len<'a, T>( + data: *const c_void, + len: usize, + ) -> CLResult<&'a [T]> { + let size = mem::size_of::(); + if len % size != 0 { + return Err(CL_INVALID_VALUE); + } + + let len = len / size; + unsafe { self::from_raw_parts(data.cast(), len) } + } + /// Wrapper around [`std::slice::from_raw_parts_mut`] that returns `Err(CL_INVALID_VALUE)` if any of these conditions is met: /// - `data` is null /// - `data` is not correctly aligned for `T`