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 <[email protected]> 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/[email protected]?part=1
