Re: [PATCH 2/4] fixup! drm/nouveau: use hmm_range_fault_unlocked_timeout() for SVM faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of the absolute timeout in nouveau_range_fault() allows an attacker to cause an infinite loop in the global GPU page fault worker thread, stalling all SVM faults.
--

--- Patch [2]: [PATCH 2/4] fixup! drm/nouveau: use hmm_range_fault_unlocked_timeout() for SVM faults ---
commit acde5c43961cd86d28658d26f11cdfcde5a9ad23
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>

fixup! drm/nouveau: use hmm_range_fault_unlocked_timeout() for SVM faults

This commit updates nouveau_range_fault() to pass the full relative timeout
to hmm_range_fault_unlocked_timeout() on each attempt. It also removes the
open-coded absolute deadline and remaining-time calculation, replacing the
bounded while loop with a goto-based retry loop.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 4cfb6eb7c7715..b1415c2e49fcc 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
[ ... ]
> @@ -677,25 +676,16 @@ static int nouveau_range_fault(struct nouveau_svmm *svmm,
>  	range.start = notifier->notifier.interval_tree.start;
>  	range.end = notifier->notifier.interval_tree.last + 1;
>  
> -	while (true) {
> -		if (time_after(jiffies, timeout)) {
> -			ret = -EBUSY;
> -			goto out;
> -		}
> -
> -		ret = hmm_range_fault_unlocked_timeout(&range,
> -						       max(timeout - jiffies,
> -							   1L));
> -		if (ret)
> -			goto out;
> +again:
> +	ret = hmm_range_fault_unlocked_timeout(&range, timeout);
> +	if (ret)
> +		goto out;
>  
> -		mutex_lock(&svmm->mutex);
> -		if (mmu_interval_read_retry(range.notifier,
> -					    range.notifier_seq)) {
> -			mutex_unlock(&svmm->mutex);
> -			continue;
> -		}
> -		break;
> +	mutex_lock(&svmm->mutex);
> +	if (mmu_interval_read_retry(range.notifier,
> +				    range.notifier_seq)) {
> +		mutex_unlock(&svmm->mutex);
> +		goto again;

[Severity: High]
Can this sequence lead to an infinite loop and stall the global GPU page fault
worker thread?

If a user-space program triggers an SVM page fault and simultaneously runs
memory invalidations like madvise(MADV_DONTNEED) in a loop on the same
memory range, mmu_interval_read_retry() will continuously return true.

Since this runs in a kernel worker thread, fatal_signal_pending(current)
inside hmm_range_fault_unlocked_timeout() will check the worker thread rather
than the user process. With the absolute timeout removed from
nouveau_range_fault(), does this allow an attacker to loop here indefinitely
and block all other SVM faults?

>  	}
>  
>  	nouveau_hmm_convert_pfn(drm, &range, args);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178405975214.1082778.5193079941156341151.stgit@skinsburskii?part=2