Re: [PATCH v2] accel/ivpu: Use separate flag for job timeout

["Wachowski, Karol" <karol.wachowski@linux.intel.com>]

On 14-Sep-26 10:58, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Lockless `xa_load` without RCU read lock and RCU-deferred freeing leads to a potential Use-After-Free of `file_priv`.
> --
> 
> commit 267f0542c58f023dad2c373ec173ea2fa96d1b24
> Author: Jakub Pawlak <jakub.pawlak@intel.com>
> 
> accel/ivpu: Use separate flag for job timeout
>      
> Use separate flag to mark a job timeout as a reason
> of starting context_abort_work. This allows to distinguish
> engine reset reason and clearly adjust reset procedure flow.
> 
>> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
>> index 41efd8985fa67..b2025274f91de 100644
>> --- a/drivers/accel/ivpu/ivpu_mmu.c
>> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> [ ... ]
>> @@ -964,7 +964,6 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
>>   		file_priv = xa_load(&vdev->context_xa, ssid);
>>   		if (file_priv) {
>>   			if (!READ_ONCE(file_priv->has_mmu_faults)) {
>> -				atomic_set(&vdev->faults_detected, 1);
>>   				ivpu_mmu_dump_event(vdev, event);
>>   				WRITE_ONCE(file_priv->has_mmu_faults, true);
>>   			}
> 
> [Severity: High]
> This is a pre-existing issue, but is it possible for file_priv to be freed
> concurrently while it is being accessed here?
> 
> xa_load() internally acquires and releases rcu_read_lock(), so the returned
> file_priv pointer is unprotected once xa_load() returns.
> 
> If a userspace application triggers an MMU fault and immediately closes the
> file descriptor, could ivpu_postclose() drop the last reference and invoke
> file_priv_release() before the hardware event queue is processed?
> 
> If file_priv_release() erases the entry from the xarray via xa_erase_irq()
> and frees it synchronously via kfree(), does this lead to a use-after-free
> when ivpu_mmu_irq_evtq_handler() subsequently accesses
> file_priv->has_mmu_faults?
> 

Thanks for the review.

Right, the race is real: xa_load() returns file_priv without
any protection, and the entry can be erased and the object freed before
has_mmu_faults is updated.

However, this is pre-existing and orthogonal to this patch: v2 only
removes atomic_set(&vdev->faults_detected, 1) from that block, so it
neither introduces nor worsens the race.

I will address it in a separate patch, with a proper Fixes: tag and
Cc: stable@vger.kernel.

At the same time I would like to proceed with this patch.

Thanks,
Karol