Re: [PATCH V3 14/19] accel/amdxdna: Move HMM invalidate wait into common GEM code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] amdxdna: hardware DMA into freed memory due to interruptible wait in hmm_invalidate
--

commit c5bcbac3bd594f37cab09ce7ecaa29fea4a1e25a
Author: David Zhang <yidong.zhang@amd.com>

accel/amdxdna: Move HMM invalidate wait into common GEM code

This commit moves the HMM invalidate wait for user mappings into the common
GEM code within amdxdna_hmm_invalidate(). Devices that don't attach fences
to user BOs will return from the wait immediately.

> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index f4832337ec31d..eebd93b6f2c31 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c

[ ... ]

> @@ -249,8 +251,12 @@ static bool amdxdna_hmm_invalidate(struct mmu_interval_notifier *mni,
>  	mmu_interval_set_seq(&mapp->notifier, cur_seq);
>  	up_write(&xdna->notifier_lock);
>  
> -	if (xdna->dev_info->ops->hmm_invalidate)
> -		xdna->dev_info->ops->hmm_invalidate(abo, cur_seq);
> +	ret = dma_resv_wait_timeout(to_gobj(abo)->resv, DMA_RESV_USAGE_BOOKKEEP,
> +				    true, MAX_SCHEDULE_TIMEOUT);
> +	if (!ret)
> +		XDNA_ERR(xdna, "Failed to wait for bo, ret %ld", ret);
> +	else if (ret == -ERESTARTSYS)
> +		XDNA_DBG(xdna, "Wait for bo interrupted by signal");
>  
>  	if (range->event == MMU_NOTIFY_UNMAP) {
>  		down_write(&xdna->notifier_lock);

[Severity: High]
Does this interruptible wait allow hardware to DMA into freed memory?

amdxdna_hmm_invalidate() is called by the OS memory subsystem when unmapping
or reclaiming memory. The driver is expected to wait for the hardware to
finish accessing the pages before returning true.

However, dma_resv_wait_timeout() is called with intr=true, which makes the
wait interruptible by signals. If the process receives a signal, the wait
aborts early and returns -ERESTARTSYS. The callback logs a debug message
but then proceeds to return true, which tells the OS that the device is no
longer accessing the memory.

Since the hardware job is still executing when the OS frees the physical
pages, can this lead to memory corruption when the hardware DMAs into those
pages?

Should this wait use intr=false to be uninterruptible in this context?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008032348.2044667-1-yidong.zhang@amd.com?part=14