[PATCH] accel/amdxdna: return early from a zero-length flush

[PATCH] accel/amdxdna: return early from a zero-length flush

[Taimuraz Kaitmazov]

SYNC_BO does not constrain its size, so a request for zero bytes reaches
drm_clflush_virt_range(), which ends with an unconditional
clflushopt(end - 1). For an empty range that is the byte before the
mapping, and abo->mem.kva comes from vmap(), so the access lands in the
guard page below the vmalloc area and faults:

  BUG: unable to handle page fault for address: ffffd16fbbc70fff
  #PF: supervisor read access in kernel mode
  Oops: Oops: 0000 [#1] SMP NOPTI
  CPU: 7 UID: 1000 Comm: sync_bo_probe
  RIP: 0010:drm_clflush_virt_range+0x3c/0x70
  Call Trace:
   amdxdna_drm_sync_bo_ioctl+0x124/0x430 [amdxdna]
   drm_ioctl+0x301/0x4c0
   __x64_sys_ioctl+0x115/0x2f0
   do_syscall_64+0xa6/0x3d0

Any process that can open the render node can do this. Reproduced 3 of 3
times on a Strix Point NPU (1022:17f0), by calling SYNC_BO with size 0 on
an AMDXDNA_BO_SHARE object. The import arm takes the same request but
flushes the whole scatterlist, so it survives it.

Nothing needs flushing for an empty range, so answer before choosing a
path.

Fixes: e252e3f3488a ("accel/amdxdna: Revise device bo creation and free")
Cc: stable@vger.kernel.org
Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
---
Trees before amdxdna_flush_bo() existed carry the same call inline in
amdxdna_drm_sync_bo_ioctl(), with args->size passed to
drm_clflush_virt_range() unclamped, so a backport wants the guard at that
call site instead.

 drivers/accel/amdxdna/amdxdna_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 1c63eff0a4a8..2a16de96e6a4 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -1247,6 +1247,9 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
 		return -EINVAL;
 
 	size = min(abo->mem.size, end) - offset;
+	if (!size)
+		return 0;
+
 	if (is_import_bo(abo))
 		drm_clflush_sg(abo->base.sgt);
 	else if (amdxdna_gem_vmap(abo))
-- 
2.55.0

Re: [PATCH] accel/amdxdna: return early from a zero-length flush

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling drm_clflush_virt_range with length 0 causes a page fault, and this can be triggered from aie2_cmdlist_multi_execbuf.
- [High] drm_clflush_virt_range() performs an unconditional out-of-bounds read when called with a length of 0, triggering a kernel panic.
- [High] Flushing a DEV BO iterates over its heaps and flushes them without pinning them, which can lead to a use-after-free if the DRM shrinker reclaims the heap's pages.
- [Medium] Userspace can trigger a kernel WARN (and potential panic if panic_on_warn is set) by passing invalid parameters to the SYNC_BO ioctl.
--

--- Patch [1]: [PATCH] accel/amdxdna: return early from a zero-length flush ---
commit 0c882e06a9bf65285c4bb50a3200aa5bf9fa60e9
Author: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>

accel/amdxdna: return early from a zero-length flush

This commit fixes a kernel panic triggered when a zero-length flush request
reaches drm_clflush_virt_range(), which accesses the byte before the mapping.
It avoids the crash by returning early from amdxdna_flush_bo() if the size is
zero.

> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index b01f5d95fac83..d18de7eb7af47 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1253,6 +1253,9 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
>  		return -EINVAL;
>  
>  	size = min(abo->mem.size, end) - offset;
> +	if (!size)
> +		return 0;
> +

[Severity: High]
This is a pre-existing issue, but by only guarding the SYNC_BO path here,
is there still a vulnerable path in aie2_cmdlist_multi_execbuf()?

If unprivileged userspace submits an ERT_CMD_CHAIN command with 
payload->command_count == 0, the offset remains 0. The function
aie2_cmdlist_multi_execbuf() in drivers/accel/amdxdna/aie2_message.c
bypasses its inner loop and directly calls:

    drm_clflush_virt_range(cmd_buf, offset);
    
Since offset is 0, this will bypass amdxdna_flush_bo() entirely and still 
trigger the kernel panic in drm_clflush_virt_range().

[Severity: High]
This isn't a bug introduced by this patch, but while fixing the zero-length 
flush here, does the root cause in drm_clflush_virt_range() need addressing 
as well?

drm_clflush_virt_range() in drivers/gpu/drm/drm_cache.c does:

    clflushopt(end - 1); /* force serialisation */
    
Since it unconditionally accesses memory prior to the buffer, calling it with 
a length of 0 causes a guard page fault. Any kernel caller that legitimately 
flushes a zero-length region can crash the system.

[Severity: High]
This is a pre-existing issue, but can flushing a DEV BO lead to a 
use-after-free?

When flushing a DEV BO, amdxdna_drm_sync_bo_ioctl() iterates over its heaps 
and calls:

    ret = amdxdna_flush_bo(heap, start - heap_start, end - start);
    
This happens without first pinning the heap BO. Later in amdxdna_flush_bo():

    drm_clflush_pages(abo->base.pages, abo->mem.size >> PAGE_SHIFT);
    
This accesses the unpinned pages array locklessly. If memory pressure invokes 
the DRM shmem shrinker concurrently, it could unpopulate and free the BO's 
pages, causing the flush operation to access freed memory.

[Severity: Medium]
This is a pre-existing issue, but can userspace trigger a kernel WARN by 
passing an out-of-bounds offset?

If an unprivileged user calls the SYNC_BO ioctl with an args->offset larger 
than the BO size, amdxdna_flush_bo() safely catches the bounds violation 
and returns -EINVAL.

However, back in amdxdna_drm_sync_bo_ioctl() in 
drivers/accel/amdxdna/amdxdna_gem.c:

    if (ret) {
        drm_WARN(&xdna->ddev, 1, "Can not get flush memory");
    }
    
This translates the user-provided bounds violation into a kernel WARN, which 
can result in a Denial of Service on systems configured with panic_on_warn.

>  	if (is_import_bo(abo))
>  		drm_clflush_sg(abo->base.sgt);
>  	else if (amdxdna_gem_vmap(abo))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817230655.356785-1-taimuraz@kaitmazov.com?part=1

Re: [PATCH] accel/amdxdna: return early from a zero-length flush

[Lizhi Hou]

On 8/17/26 16:06, Taimuraz Kaitmazov wrote:
> SYNC_BO does not constrain its size, so a request for zero bytes reaches
> drm_clflush_virt_range(), which ends with an unconditional
> clflushopt(end - 1). For an empty range that is the byte before the
> mapping, and abo->mem.kva comes from vmap(), so the access lands in the
> guard page below the vmalloc area and faults:
>
>    BUG: unable to handle page fault for address: ffffd16fbbc70fff
>    #PF: supervisor read access in kernel mode
>    Oops: Oops: 0000 [#1] SMP NOPTI
>    CPU: 7 UID: 1000 Comm: sync_bo_probe
>    RIP: 0010:drm_clflush_virt_range+0x3c/0x70
>    Call Trace:
>     amdxdna_drm_sync_bo_ioctl+0x124/0x430 [amdxdna]
>     drm_ioctl+0x301/0x4c0
>     __x64_sys_ioctl+0x115/0x2f0
>     do_syscall_64+0xa6/0x3d0
>
> Any process that can open the render node can do this. Reproduced 3 of 3
> times on a Strix Point NPU (1022:17f0), by calling SYNC_BO with size 0 on
> an AMDXDNA_BO_SHARE object. The import arm takes the same request but
> flushes the whole scatterlist, so it survives it.
>
> Nothing needs flushing for an empty range, so answer before choosing a
> path.
>
> Fixes: e252e3f3488a ("accel/amdxdna: Revise device bo creation and free")
> Cc: stable@vger.kernel.org
> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
> ---
> Trees before amdxdna_flush_bo() existed carry the same call inline in
> amdxdna_drm_sync_bo_ioctl(), with args->size passed to
> drm_clflush_virt_range() unclamped, so a backport wants the guard at that
> call site instead.
>
>   drivers/accel/amdxdna/amdxdna_gem.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 1c63eff0a4a8..2a16de96e6a4 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -1247,6 +1247,9 @@ static int amdxdna_flush_bo(struct amdxdna_gem_obj *abo, u64 offset, u64 size)
>   		return -EINVAL;
>   
>   	size = min(abo->mem.size, end) - offset;
> +	if (!size)
> +		return 0;
> +
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>   	if (is_import_bo(abo))
>   		drm_clflush_sg(abo->base.sgt);
>   	else if (amdxdna_gem_vmap(abo))

Re: [PATCH] accel/amdxdna: return early from a zero-length flush

[Lizhi Hou]

Applied to drm-misc-fixes

On 8/18/26 09:23, Lizhi Hou wrote:
>
> On 8/17/26 16:06, Taimuraz Kaitmazov wrote:
>> SYNC_BO does not constrain its size, so a request for zero bytes reaches
>> drm_clflush_virt_range(), which ends with an unconditional
>> clflushopt(end - 1). For an empty range that is the byte before the
>> mapping, and abo->mem.kva comes from vmap(), so the access lands in the
>> guard page below the vmalloc area and faults:
>>
>>    BUG: unable to handle page fault for address: ffffd16fbbc70fff
>>    #PF: supervisor read access in kernel mode
>>    Oops: Oops: 0000 [#1] SMP NOPTI
>>    CPU: 7 UID: 1000 Comm: sync_bo_probe
>>    RIP: 0010:drm_clflush_virt_range+0x3c/0x70
>>    Call Trace:
>>     amdxdna_drm_sync_bo_ioctl+0x124/0x430 [amdxdna]
>>     drm_ioctl+0x301/0x4c0
>>     __x64_sys_ioctl+0x115/0x2f0
>>     do_syscall_64+0xa6/0x3d0
>>
>> Any process that can open the render node can do this. Reproduced 3 of 3
>> times on a Strix Point NPU (1022:17f0), by calling SYNC_BO with size 
>> 0 on
>> an AMDXDNA_BO_SHARE object. The import arm takes the same request but
>> flushes the whole scatterlist, so it survives it.
>>
>> Nothing needs flushing for an empty range, so answer before choosing a
>> path.
>>
>> Fixes: e252e3f3488a ("accel/amdxdna: Revise device bo creation and 
>> free")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>
>> ---
>> Trees before amdxdna_flush_bo() existed carry the same call inline in
>> amdxdna_drm_sync_bo_ioctl(), with args->size passed to
>> drm_clflush_virt_range() unclamped, so a backport wants the guard at 
>> that
>> call site instead.
>>
>>   drivers/accel/amdxdna/amdxdna_gem.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 1c63eff0a4a8..2a16de96e6a4 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -1247,6 +1247,9 @@ static int amdxdna_flush_bo(struct 
>> amdxdna_gem_obj *abo, u64 offset, u64 size)
>>           return -EINVAL;
>>         size = min(abo->mem.size, end) - offset;
>> +    if (!size)
>> +        return 0;
>> +
> Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
>>       if (is_import_bo(abo))
>>           drm_clflush_sg(abo->base.sgt);
>>       else if (amdxdna_gem_vmap(abo))