On 14-Sep-26 10:58, [email protected] 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 <[email protected]>
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: [email protected].
At the same time I would like to proceed with this patch.
Thanks,
Karol
