[PATCH v2] drm/amdgpu: add the BO-va mapping offset when kmapping an IB

[PATCH v2] drm/amdgpu: add the BO-va mapping offset when kmapping an IB

[Junrui Luo via B4 Relay]

From: Junrui Luo <moonafterrain@outlook.com>

amdgpu_cs_patch_ibs() derives the CPU-side view of a UVD/VCE/VCN
indirect buffer from the BO returned by amdgpu_cs_find_mapping():

    r = amdgpu_bo_kmap(aobj, (void **)&kptr);
    kptr += va_start - (m->start * AMDGPU_GPU_PAGE_SIZE);

amdgpu_bo_kmap() returns the start of the BO, so only the displacement
of va_start inside the mapping is added.  The page tables, however, are
programmed from mapping->offset (see amdgpu_vm_bo_update()), which
records the offset_in_bo the client passed to AMDGPU_GEM_VA.  The GPU
therefore resolves va_start to BO byte

    m->offset + (va_start - m->start * AMDGPU_GPU_PAGE_SIZE)

while the kernel inspects the byte m->offset lower.  Whenever an IB is
submitted through a mapping created with a non-zero offset_in_bo, the
two views disagree.

Add the missing term so the kmapped pointer describes the same bytes the
page tables do.

Every other CPU-side consumer of amdgpu_cs_find_mapping() omits
mapping->offset in the same way.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
Changes in v2:
- Drop the Fixes tag.
- Pick up Christian's Reviewed-by.
- Resend standalone.
- Link to v1: https://lore.kernel.org/r/20260806-amdgpu-fixes-v1-3-ce247012d4da@outlook.com
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 5445f75741b5..17fe6d56e020 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);
 
 		if (ring->funcs->parse_cs) {
 			memcpy(ib->ptr, kptr, ib->length_dw * 4);

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260808-amdgpu-fixes-9ca10fa07e10

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

[PATCH v2] drm/amdgpu: add the BO-va mapping offset when kmapping an IB

[Junrui Luo]

amdgpu_cs_patch_ibs() derives the CPU-side view of a UVD/VCE/VCN
indirect buffer from the BO returned by amdgpu_cs_find_mapping():

    r = amdgpu_bo_kmap(aobj, (void **)&kptr);
    kptr += va_start - (m->start * AMDGPU_GPU_PAGE_SIZE);

amdgpu_bo_kmap() returns the start of the BO, so only the displacement
of va_start inside the mapping is added.  The page tables, however, are
programmed from mapping->offset (see amdgpu_vm_bo_update()), which
records the offset_in_bo the client passed to AMDGPU_GEM_VA.  The GPU
therefore resolves va_start to BO byte

    m->offset + (va_start - m->start * AMDGPU_GPU_PAGE_SIZE)

while the kernel inspects the byte m->offset lower.  Whenever an IB is
submitted through a mapping created with a non-zero offset_in_bo, the
two views disagree.

Add the missing term so the kmapped pointer describes the same bytes the
page tables do.

Every other CPU-side consumer of amdgpu_cs_find_mapping() omits
mapping->offset in the same way.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
Changes in v2:
- Drop the Fixes tag.
- Pick up Christian's Reviewed-by.
- Resend standalone.
- Link to v1: https://lore.kernel.org/r/20260806-amdgpu-fixes-v1-3-ce247012d4da@outlook.com
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 5445f75741b5..17fe6d56e020 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);
 
 		if (ring->funcs->parse_cs) {
 			memcpy(ib->ptr, kptr, ib->length_dw * 4);

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260808-amdgpu-fixes-9ca10fa07e10

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH v2] drm/amdgpu: add the BO-va mapping offset when kmapping an IB

[sashiko-bot]

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 <moonafterrain@outlook.com>

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/20260806-amdgpu-fixes-v1-3-ce247012d4da@outlook.com

> 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/20260808-amdgpu-fixes-v2-1-36d66398601f@outlook.com?part=1