Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Critical] The patch introduces a kernel memory out-of-bounds read/write vulnerability on architectures where the system page size is greater than 4KB (e.g., 64KB on ARM64 or PPC64) due to a pre-existing shift bug. Pre-existing issues: - [High] The patch incompletely fixes the `mapping->offset` omission. While it fixes `amdgpu_cs_patch_ibs()`, it explicitly ignores other CPU-side consumers of `amdgpu_cs_find_mapping()` (VCN, UVD, and VCE) that suffer from the identical bug, leaving a TOCTOU validation bypass and hardware programming errors. -- --- Patch [1]: [PATCH v2] drm/amdgpu: add the BO-va mapping offset when kmapping an IB --- commit 38c532743ee774da4df8cab806dff666d610d7a1 Author: Junrui Luo <[email protected]> drm/amdgpu: add the BO-va mapping offset when kmapping an IB This commit updates amdgpu_cs_patch_ibs() to add the m->offset term when calculating the kmapped pointer. This ensures the CPU and GPU views of the indirect buffer align when a mapping is created with a non-zero offset. Link: https://lore.kernel.org/r/[email protected] > Every other CPU-side consumer of amdgpu_cs_find_mapping() omits > mapping->offset in the same way. [Severity: High] This is a pre-existing issue, but does vcn_v3_0_dec_msg() calculate the CPU pointer without including map->offset? drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c:vcn_v3_0_dec_msg() { ... msg = ptr + addr - start; ... } Could similar regressions be present in vcn_v4_0_dec_msg(), amdgpu_uvd_cs_pass2(), and amdgpu_vce_cs_reloc() where the physical GPU address or CPU pointer is calculated without the mapping offset? > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 5445f75741b53..17fe6d56e0206 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -1043,7 +1043,7 @@ static int amdgpu_cs_patch_ibs(struct amdgpu_cs_parser > *p, > if (r) > return r; > > - kptr += va_start - (m->start * AMDGPU_GPU_PAGE_SIZE); > + kptr += m->offset + va_start - (m->start * > AMDGPU_GPU_PAGE_SIZE); [Severity: Critical] Can this calculation lead to an out-of-bounds access if m->offset is inflated during a mapping split? For example, if a pre-existing shift bug occurs on architectures with a system page size greater than 4KB, could adding m->offset push kptr out of bounds before it is used? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
