CREATE_QUEUE takes an unprivileged, only lower-bounded ctx_save_restore_area_size. kfd_queue_acquire_buffers() computes the total CWSR size as (ctx_save_restore_area_size + debug_memory_size) * NUM_XCC() in a u32 with no overflow check. On multi-XCC parts (NUM_XCC up to 8) a large size wraps to a small value, so an undersized BO passes validation while init_mqd_v9_4_3() still strides by the raw size -- a GPU-side out-of-bounds write from an unprivileged /dev/kfd caller.
Compute the total with check_add/mul_overflow() and reject on overflow, and bound-check the per-XCC CWSR stride in the MQD managers. Signed-off-by: Yongqiang Sun <[email protected]> --- .../drm/amd/amdkfd/kfd_mqd_manager_v12_1.c | 8 ++- .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 8 ++- drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 31 ++++++++++++ drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 49 ++++++++++++++++--- 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c index a6fff8032dce..d45b3fc3baec 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c @@ -473,7 +473,8 @@ static void init_mqd_v12_1(struct mqd_manager *mm, void **mqd, * and CWSR area is allocated in thunk */ if (mm->dev->kfd->cwsr_enabled && - q->ctx_save_restore_area_address) { + q->ctx_save_restore_area_address && + !WARN_ON_ONCE(!kfd_queue_cwsr_xcc_in_bounds(q, xcc))) { xcc_ctx_save_restore_area_address = q->ctx_save_restore_area_address + (xcc * q->ctx_save_restore_area_size); @@ -596,6 +597,11 @@ static int get_wave_state_v12_1(struct mqd_manager *mm, void *mqd, u32 tmp_ctl_stack_used_size = 0, tmp_save_area_used_size = 0; for (xcc = 0; xcc < NUM_XCC(mm->dev->xcc_mask); xcc++) { + if (WARN_ON_ONCE(!kfd_queue_cwsr_xcc_in_bounds(q, xcc))) { + err = -EINVAL; + break; + } + xcc_mqd = mqd + mqd_stride_size * xcc; xcc_ctl_stack = (void __user *)((uintptr_t)ctl_stack + q->ctx_save_restore_area_size * xcc); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c index ce379ab17916..83a4cb67c01f 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c @@ -727,7 +727,8 @@ static void init_mqd_v9_4_3(struct mqd_manager *mm, void **mqd, * and CWSR area is allocated in thunk */ if (mm->dev->kfd->cwsr_enabled && - q->ctx_save_restore_area_address) { + q->ctx_save_restore_area_address && + !WARN_ON_ONCE(!kfd_queue_cwsr_xcc_in_bounds(q, xcc))) { xcc_ctx_save_restore_area_address = q->ctx_save_restore_area_address + (xcc * q->ctx_save_restore_area_size); @@ -918,6 +919,11 @@ static int get_wave_state_v9_4_3(struct mqd_manager *mm, void *mqd, u32 tmp_ctl_stack_used_size = 0, tmp_save_area_used_size = 0; for (xcc = 0; xcc < NUM_XCC(mm->dev->xcc_mask); xcc++) { + if (WARN_ON_ONCE(!kfd_queue_cwsr_xcc_in_bounds(q, xcc))) { + err = -EINVAL; + break; + } + xcc_mqd = mqd + mqd_stride_size * xcc; xcc_ctl_stack = (void __user *)((uintptr_t)ctl_stack + q->ctx_save_restore_area_size * xcc); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h index ad4897f094a2..f227942f50ec 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h @@ -540,6 +540,14 @@ struct queue_properties { uint32_t eop_ring_buffer_size; uint64_t ctx_save_restore_area_address; uint32_t ctx_save_restore_area_size; + /* + * Total, overflow-checked size of the CWSR buffer across all XCCs as + * validated against the pinned BO in kfd_queue_acquire_buffers(). The + * MQD managers use this as an upper bound when programming the per-XCC + * CWSR save base so the raw per-XCC stride can never index past the + * validated allocation. + */ + uint32_t cwsr_area_total_size; uint32_t ctl_stack_size; uint64_t tba_addr; uint64_t tma_addr; @@ -552,6 +560,29 @@ struct queue_properties { struct amdgpu_bo *cwsr_bo; }; +/* + * Bound-check a per-XCC CWSR save area against the validated total CWSR buffer + * size. The MQD managers program the per-XCC CWSR base using the raw + * ctx_save_restore_area_size as the stride; this confirms the save area for + * @xcc lies within the allocation that kfd_queue_acquire_buffers() validated. + * + * Returns true when the access is in bounds. A false return indicates a kernel + * bug (the overflow-checked validation in kfd_queue_acquire_buffers() should + * have already rejected the queue), and the caller must not program an + * out-of-bounds CWSR base. When no validated size was recorded + * (cwsr_area_total_size == 0, e.g. non-CWSR queues) the legacy behavior is + * preserved. + */ +static inline bool kfd_queue_cwsr_xcc_in_bounds(const struct queue_properties *q, + unsigned int xcc) +{ + if (!q->cwsr_area_total_size) + return true; + + return (u64)(xcc + 1) * q->ctx_save_restore_area_size <= + q->cwsr_area_total_size; +} + #define QUEUE_IS_ACTIVE(q) ((q).queue_size > 0 && \ (q).queue_address != 0 && \ (q).queue_percent > 0 && \ diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c index 9d4838461168..6bb7aa0fdf1a 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c @@ -23,6 +23,7 @@ */ #include <linux/slab.h> +#include <linux/overflow.h> #include "kfd_priv.h" #include "kfd_topology.h" #include "kfd_svm.h" @@ -230,6 +231,40 @@ void kfd_queue_buffer_put(struct amdgpu_bo **bo) amdgpu_bo_unref(bo); } +/* + * Compute the total CWSR buffer size across all XCCs. + * + * The per-XCC size and the NUM_XCC multiply are both attacker-influenced + * (ctx_save_restore_area_size comes straight from the CREATE_QUEUE ioctl and + * is only lower-bounded), so the arithmetic must be overflow-checked. A wrapped + * result would let an undersized BO pass the exact-match validation in + * kfd_queue_buffer_get() while the per-XCC CWSR hardware stride still uses the + * raw user size, causing a GPU-side out-of-bounds write on multi-XCC parts. + */ +static int kfd_queue_compute_cwsr_size(struct kfd_process_device *pdd, + struct kfd_topology_device *topo_dev, + struct queue_properties *properties, + u32 *total_cwsr_size) +{ + u32 per_xcc_size; + + if (check_add_overflow(properties->ctx_save_restore_area_size, + topo_dev->node_props.debug_memory_size, + &per_xcc_size)) + return -EINVAL; + + if (check_mul_overflow(per_xcc_size, NUM_XCC(pdd->dev->xcc_mask), + total_cwsr_size)) + return -EINVAL; + + /* PAGE_ALIGN() must not wrap either. */ + if (*total_cwsr_size > U32_MAX - (PAGE_SIZE - 1)) + return -EINVAL; + *total_cwsr_size = ALIGN(*total_cwsr_size, PAGE_SIZE); + + return 0; +} + int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_properties *properties) { struct kfd_topology_device *topo_dev; @@ -308,9 +343,12 @@ int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct queue_prope goto out_err_unreserve; } - total_cwsr_size = (properties->ctx_save_restore_area_size + - topo_dev->node_props.debug_memory_size) * NUM_XCC(pdd->dev->xcc_mask); - total_cwsr_size = ALIGN(total_cwsr_size, PAGE_SIZE); + err = kfd_queue_compute_cwsr_size(pdd, topo_dev, properties, &total_cwsr_size); + if (err) + goto out_err_unreserve; + + /* Carry the validated size to the MQD managers as a per-XCC bound. */ + properties->cwsr_area_total_size = total_cwsr_size; err = kfd_queue_buffer_get(vm, (void *)properties->ctx_save_restore_area_address, &properties->cwsr_bo, total_cwsr_size); @@ -355,9 +393,8 @@ int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct queue_prope topo_dev = kfd_topology_device_by_id(pdd->dev->id); if (!topo_dev) return -EINVAL; - total_cwsr_size = (properties->ctx_save_restore_area_size + - topo_dev->node_props.debug_memory_size) * NUM_XCC(pdd->dev->xcc_mask); - total_cwsr_size = ALIGN(total_cwsr_size, PAGE_SIZE); + if (kfd_queue_compute_cwsr_size(pdd, topo_dev, properties, &total_cwsr_size)) + return -EINVAL; kfd_queue_buffer_svm_put(pdd, properties->ctx_save_restore_area_address, total_cwsr_size); return 0; -- 2.43.0
