Re: [PATCH AUTOSEL 6.18-6.6] drm/amdkfd: Fix OOB memory exposure in get_wave_state()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unclamped `m->cp_hqd_cntl_stack_size` in `checkpoint_mqd` paths allows heap buffer overflow and kernel memory leak.
--

commit 5f7a303c0f081c68cdc169627eeb4062bb061165
Author: Sunday Clement <Sunday.Clement@amd.com>

drm/amdkfd: Fix OOB memory exposure in get_wave_state()

This commit fixes an out-of-bounds memory exposure in get_wave_state() by
clamping the attacker-controlled cp_hqd_cntl_stack_size and offset values
from the MQD. The fix ensures that copy_to_user() does not read beyond the
allocated kernel buffer object.

> 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 f2dee320fada4..e3a33a92b1799 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c

[ ... ]

While this patch fixes the out-of-bounds read in get_wave_state(), I noticed a
similar pattern in the checkpointing paths further down in the file.

drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c:get_checkpoint_info() {
	struct v9_mqd *m = get_mqd(mqd);

	*ctl_stack_size = m->cp_hqd_cntl_stack_size * NUM_XCC(mm->dev->xcc_mask);
}

[Severity: High]
This is a pre-existing issue, but does this calculate the buffer allocation
size using an untrusted, unclamped field from XCC 0? Could an attacker with
CAP_CHECKPOINT_RESTORE use a maliciously crafted MQD payload during
AMDKFD_IOC_CRIU_OP to cause an under-allocation here?

drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c:checkpoint_mqd() {
	...
	memcpy(ctl_stack_dst, ctl_stack, m->cp_hqd_cntl_stack_size);
}

[Severity: High]
This is a pre-existing issue, but could this perform an unbounded read from
the kernel BO and an unbounded write into the heap buffer based on the
unclamped attacker-controlled size?

drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c:checkpoint_mqd_v9_4_3() {
	...
	for (xcc = 0; xcc < NUM_XCC(mm->dev->xcc_mask); xcc++) {
		m = get_mqd(mqd + size * xcc);

		checkpoint_mqd(mm, m,
				(uint8_t *)mqd_dst + sizeof(*m) * xcc,
				(uint8_t *)ctl_stack_dst + m->cp_hqd_cntl_stack_size * xcc);
	}
}

[Severity: High]
This is a pre-existing issue, but in multi-XCC environments (e.g., v9.4.3),
if a subsequent XCC has a much larger cp_hqd_cntl_stack_size than XCC 0, does
passing this unverified size to the memcpy wrapper trigger a heap overflow?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-392-sashal@kernel.org?part=1