On Sat, Aug 22, 2026 at 2:08 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
>
> Pre-existing issues:
> - [High] Sleeping in atomic context
> - [High] Deferred teardown TOCTOU race via flink name re-opening causing IOVA
> Use-After-Free
> --
>
> --- Patch [2]: [PATCH 2/2] drm/msm: don't tear down shared VM mappings on
> handle close ---
> commit c4a550033efcf938dc9587175385266d18df3979
> Author: Dmitry Baryshkov <[email protected]>
>
> drm/msm: don't tear down shared VM mappings on handle close
>
> This patch defers the teardown of shared GPU VM mappings until the vma_ref
> count drops to zero. This aims to prevent pulling the buffer out from under
> other files that still have it open when sharing a global VM address space
> on targets without per-process pgtables.
>
> Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion")
>
> > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > index 8b979d84bb958..f17ecd97b72a3 100644
> > --- a/drivers/gpu/drm/msm/msm_gem.c
> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> [ ... ]
> > @@ -109,6 +114,12 @@ void msm_gem_vma_put(struct drm_gem_object *obj)
> > if (atomic_dec_return(&to_msm_bo(obj)->vma_ref))
> > return;
> >
> > + if (priv->gpu && priv->gpu->vm_shared) {
> > + dma_resv_wait_timeout(obj->resv, DMA_RESV_USAGE_BOOKKEEP,
> > false,
> > + MAX_SCHEDULE_TIMEOUT);
> > + put_iova_spaces(obj, priv->gpu->vm, true, "vma_put");
> > + }
> > +
>
> [Severity: High]
> This is a pre-existing issue, but can this lockless vma_ref check lead to an
> IOVA use-after-free if a GEM handle is concurrently closed and re-opened via
> flink?
>
> If Thread A closes the last handle, atomic_dec_return() drops vma_ref to 0
> and it proceeds to unmap the buffer. However, the flink name remains valid
> until drm_gem_object_handle_put_unlocked() runs. If Thread B calls
> DRM_IOCTL_GEM_OPEN concurrently, msm_gem_open() can increment vma_ref back to
> 1 and submit a job using the IOVA. Thread A then acquires locks in
> put_iova_spaces() and unconditionally unmaps the buffer via
> put_iova_spaces_locked() without re-verifying vma_ref:
This is already invalid, the flink name doesn't hold a ref to the obj.
The exporter must hold the obj live (by not closing the handle) of an
flink exported BO.
>
> drivers/gpu/drm/msm/msm_gem.c:put_iova_spaces_locked() {
> ...
> drm_gpuvm_bo_for_each_va_safe (vma, vmatmp, vm_bo) {
> msm_gem_vma_unmap(vma, reason);
> if (close)
> msm_gem_vma_close(vma);
> }
> }
>
> Will this destroy Thread B's mapping while its job is running, freeing the
> IOVA for immediate reuse?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does adding these blocking
> calls cause a scheduling while atomic panic when tearing down a writeback
> connector?
>
> The drm core function drm_writeback_connector_cleanup() holds a spinlock with
> IRQs disabled when tearing down pending jobs:
>
> drivers/gpu/drm/drm_writeback.c:drm_writeback_connector_cleanup() {
> spin_lock_irqsave(&wb_connector->job_lock, flags);
> list_for_each_entry_safe(pos, n, &wb_connector->job_queue, list_entry) {
> list_del(&pos->list_entry);
> drm_writeback_cleanup_job(pos);
> }
> }
Hmm, someone should look at this.. maybe we could move to an on-stack
list under a lock, and then do cleanup outside of the lock?
BR,
-R
>
> This calls into msm_gem_vma_put() through the teardown path
> (dpu_wb_conn_cleanup_job -> dpu_encoder_cleanup_wb_job ->
> dpu_encoder_phys_wb_cleanup_wb_job -> msm_framebuffer_cleanup ->
> msm_gem_vma_put).
>
> Since dma_resv_wait_timeout() can wait indefinitely and put_iova_spaces()
> acquires ww_mutexes via drm_exec, could this result in sleeping while holding
> a spinlock with IRQs disabled?
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=2
