get_wave_state() on gfx9 clamps the control stack size and offset before
the copy_to_user() of the control stack, but still builds the returned
ctl_stack_used_size, save_area_used_size and the context save area
header from the raw MQD values. The gfx8, gfx10, gfx11, gfx12 and
gfx12.1 versions do the same without any clamp. With an inconsistent MQD
(for example restored from a CRIU image) the header and the returned
sizes do not match what was copied, and either subtraction
(cp_hqd_cntl_stack_size - cp_hqd_cntl_stack_offset,
cp_hqd_wg_state_offset - cp_hqd_cntl_stack_size) can wrap to a value
near 4 GiB. get_wave_state_v9_4_3() and get_wave_state_v12_1() inherit
this since they call it per XCC.
Add kfd_mqd_clamp_wave_state(), which derives the header and both used
sizes from clamped values: offset <= size <= ceiling and
wg_state_offset >= size. gfx9 passes q->ctl_stack_size, the allocated
control stack size, as the ceiling. gfx8, gfx10 and newer pass U32_MAX:
they copy nothing from the MQD buffer, their control stack is part of
the user's save area. Values from a consistent MQD are unchanged, and
the header keeps its layout of four u32 fields.
wg_state_offset has no trusted upper bound: the wave state lives in the
user's save area, which the kernel does not access.
Fixes: 5df099e8bc83 ("drm/amdkfd: Add wavefront context save state retrieval
ioctl")
Fixes: 14328aa58ce5 ("drm/amdkfd: Add navi10 support to amdkfd. (v3)")
Fixes: cc009e613de6 ("drm/amdkfd: Add KFD support for soc21 v3")
Fixes: 48f0bdf4e38e ("drm/amdkfd: Added MQD manager files for GFX12.")
Fixes: 01bbc4a4b947 ("drm/amdkfd: Add MQD manager for GFX 12.1.0")
Fixes: 7ef144458f48 ("drm/amdkfd: Fix OOB memory exposure in get_wave_state()")
Signed-off-by: William Palacek <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 16 ++++++++++
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c | 19 ++++--------
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c | 18 ++++-------
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c | 14 +++------
.../drm/amd/amdkfd/kfd_mqd_manager_v12_1.c | 14 +++------
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 31 ++++++-------------
.../gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 9 +++---
7 files changed, 51 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
index 38b46b696243..dc4681328a47 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
@@ -163,6 +163,22 @@ struct mqd_user_context_save_area_header {
uint32_t wave_state_size;
};
+/*
+ * Fill the user header from raw MQD fields. They come from CP firmware or a
+ * CRIU image, so make them self-consistent: offset <= size <= ceiling and
+ * wg_state_offset >= size, which keeps both used sizes from wrapping.
+ */
+static inline void kfd_mqd_clamp_wave_state(u32 cntl_stack_size, u32
cntl_stack_offset,
+ u32 wg_state_offset, u32 ceiling,
+ struct
mqd_user_context_save_area_header *hdr)
+{
+ cntl_stack_size = min(cntl_stack_size, ceiling);
+ hdr->control_stack_offset = min(cntl_stack_offset, cntl_stack_size);
+ hdr->wave_state_offset = max(wg_state_offset, cntl_stack_size);
+ hdr->control_stack_size = cntl_stack_size - hdr->control_stack_offset;
+ hdr->wave_state_size = hdr->wave_state_offset - cntl_stack_size;
+}
+
struct kfd_mem_obj *allocate_hiq_mqd(struct mqd_manager *mm,
struct queue_properties *q);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
index 4f8a8a1a6186..64a97723c083 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
@@ -252,7 +252,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
u32 *save_area_used_size)
{
struct v10_compute_mqd *m;
- struct kfd_context_save_area_header header;
+ struct mqd_user_context_save_area_header header;
m = get_mqd(mqd);
@@ -261,23 +261,16 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
* Current position is at m->cp_hqd_cntl_stack_offset and
* m->cp_hqd_wg_state_offset, respectively.
*/
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, U32_MAX, &header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
/* Control stack is not copied to user mode for GFXv10 because
* it's part of the context save area that is already
* accessible to user mode
*/
-
- header.wave_state.control_stack_size = *ctl_stack_used_size;
- header.wave_state.wave_state_size = *save_area_used_size;
-
- header.wave_state.wave_state_offset = m->cp_hqd_wg_state_offset;
- header.wave_state.control_stack_offset = m->cp_hqd_cntl_stack_offset;
-
- if (copy_to_user(ctl_stack, &header, sizeof(header.wave_state)))
+ if (copy_to_user(ctl_stack, &header, sizeof(header)))
return -EFAULT;
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
index bf015dc5b868..e3d2f50f86c8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v11.c
@@ -289,7 +289,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
u32 *save_area_used_size)
{
struct v11_compute_mqd *m;
- struct kfd_context_save_area_header header;
+ struct mqd_user_context_save_area_header header;
m = get_mqd(mqd);
@@ -298,22 +298,16 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
* Current position is at m->cp_hqd_cntl_stack_offset and
* m->cp_hqd_wg_state_offset, respectively.
*/
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, U32_MAX, &header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
/* Control stack is not copied to user mode for GFXv11 because
* it's part of the context save area that is already
* accessible to user mode
*/
- header.wave_state.control_stack_size = *ctl_stack_used_size;
- header.wave_state.wave_state_size = *save_area_used_size;
-
- header.wave_state.wave_state_offset = m->cp_hqd_wg_state_offset;
- header.wave_state.control_stack_offset = m->cp_hqd_cntl_stack_offset;
-
- if (copy_to_user(ctl_stack, &header, sizeof(header.wave_state)))
+ if (copy_to_user(ctl_stack, &header, sizeof(header)))
return -EFAULT;
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
index 6ea09b031caf..1068a7629331 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
@@ -267,21 +267,15 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
* Current position is at m->cp_hqd_cntl_stack_offset and
* m->cp_hqd_wg_state_offset, respectively.
*/
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, U32_MAX, &header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
/* Control stack is not copied to user mode for GFXv12 because
* it's part of the context save area that is already
* accessible to user mode
*/
- header.control_stack_size = *ctl_stack_used_size;
- header.wave_state_size = *save_area_used_size;
-
- header.wave_state_offset = m->cp_hqd_wg_state_offset;
- header.control_stack_offset = m->cp_hqd_cntl_stack_offset;
-
if (copy_to_user(ctl_stack, &header, sizeof(header)))
return -EFAULT;
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 d508ac5a18f8..79e0d6250720 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
@@ -369,21 +369,15 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
* Current position is at m->cp_hqd_cntl_stack_offset and
* m->cp_hqd_wg_state_offset, respectively.
*/
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, U32_MAX, &header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
/* Control stack is not copied to user mode for GFXv12 because
* it's part of the context save area that is already
* accessible to user mode
*/
- header.control_stack_size = *ctl_stack_used_size;
- header.wave_state_size = *save_area_used_size;
-
- header.wave_state_offset = m->cp_hqd_wg_state_offset;
- header.control_stack_offset = m->cp_hqd_cntl_stack_offset;
-
if (copy_to_user(ctl_stack, &header, sizeof(header)))
return -EFAULT;
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 35c36b6dc212..948415908551 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -364,35 +364,24 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
u32 *save_area_used_size)
{
struct v9_mqd *m;
- struct kfd_context_save_area_header header;
- u32 cntl_stack_size;
- u32 cntl_stack_offset;
+ struct mqd_user_context_save_area_header header;
/* Control stack is located one page after MQD. */
void *mqd_ctl_stack = (void *)((uintptr_t)mqd + AMDGPU_GPU_PAGE_SIZE);
m = get_mqd(mqd);
- cntl_stack_size = min_t(u32, m->cp_hqd_cntl_stack_size,
q->ctl_stack_size);
- cntl_stack_offset = min_t(u32, m->cp_hqd_cntl_stack_offset,
cntl_stack_size);
+ /* Only q->ctl_stack_size, the allocation size, is trusted. */
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, q->ctl_stack_size,
&header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
-
- header.wave_state.control_stack_size = *ctl_stack_used_size;
- header.wave_state.wave_state_size = *save_area_used_size;
-
- header.wave_state.wave_state_offset = m->cp_hqd_wg_state_offset;
- header.wave_state.control_stack_offset = m->cp_hqd_cntl_stack_offset;
-
- if (copy_to_user(ctl_stack, &header, sizeof(header.wave_state)))
+ if (copy_to_user(ctl_stack, &header, sizeof(header)))
return -EFAULT;
- *ctl_stack_used_size = cntl_stack_size - cntl_stack_offset;
-
- if (copy_to_user(ctl_stack + cntl_stack_offset, mqd_ctl_stack +
cntl_stack_offset,
- *ctl_stack_used_size))
+ if (copy_to_user(ctl_stack + header.control_stack_offset,
+ mqd_ctl_stack + header.control_stack_offset,
+ *ctl_stack_used_size))
return -EFAULT;
return 0;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
index 029572548c14..a11d01689f8b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
@@ -272,13 +272,14 @@ static int get_wave_state(struct mqd_manager *mm, void
*mqd,
u32 *save_area_used_size)
{
struct vi_mqd *m;
+ struct mqd_user_context_save_area_header header;
m = get_mqd(mqd);
- *ctl_stack_used_size = m->cp_hqd_cntl_stack_size -
- m->cp_hqd_cntl_stack_offset;
- *save_area_used_size = m->cp_hqd_wg_state_offset -
- m->cp_hqd_cntl_stack_size;
+ kfd_mqd_clamp_wave_state(m->cp_hqd_cntl_stack_size,
m->cp_hqd_cntl_stack_offset,
+ m->cp_hqd_wg_state_offset, U32_MAX, &header);
+ *ctl_stack_used_size = header.control_stack_size;
+ *save_area_used_size = header.wave_state_size;
/* Control stack is not copied to user mode for GFXv8 because
* it's part of the context save area that is already
--
2.43.0
