Reviewed-by: Lyude Paul <[email protected]>
Will push to drm-misc-next in just a moment
On Fri, 2026-08-14 at 18:00 +0800, Zhenhao Wan wrote:
> The SVM fault handler nouveau_svm_fault() looks up each fault's
> nouveau_svmm under svm->mutex, caches it in the fault array, then
> drops
> svm->mutex and dereferences the svmm across blocking faults
> (mmu_interval_notifier_insert(), hmm_range_fault()).
>
> nouveau_svmm is not refcounted. On DRM file close nouveau_svmm_fini()
> ->
> mmu_notifier_put() frees it asynchronously via call_srcu() without
> waiting for the handler, and mmget_not_zero() pins the mm, not the
> svmm.
> flush_work() on the fault buffer otherwise runs only at device
> removal
> and suspend, never on close, so a fault handled concurrently with a
> close
> can dereference a freed svmm.
>
> The instance is already unlinked by nouveau_svmm_part() earlier in
> the
> same close, so no new fault can resolve to it. Drain the handler in
> nouveau_svmm_fini() before the free, guarding the flush with a NULL
> check
> on drm->svm: on device teardown nouveau_svm_fini() runs first and
> frees
> drm->svm (NULL) after blocking the notify and flushing the buffer, so
> the
> handler is already drained; on per-client close drm->svm is alive and
> the
> flush runs.
>
> Fixes: eeaf06ac1a55 ("drm/nouveau/svm: initial support for shared
> virtual memory")
> Reported-by: Yuhao Jiang <[email protected]>
> Assisted-by: Claude:claude-opus-5
> Cc: [email protected]
> Signed-off-by: Zhenhao Wan <[email protected]>
> ---
> Changes in v2:
> - Fix a NULL deref introduced in v1: guard flush_work() with a NULL
> check on drm->svm. On device teardown nouveau_svm_fini() frees
> drm->svm before the per-client svmm teardown, so
> nouveau_svmm_fini()
> would deref NULL (reported by Sashiko AI review); the handler is
> already drained there, so there is nothing left to flush.
> - Tighten the commit message and the code comment; no functional
> change.
> - Link to v1:
> https://patch.msgid.link/[email protected]
>
> To: Lyude Paul <[email protected]>
> To: Danilo Krummrich <[email protected]>
> To: Maarten Lankhorst <[email protected]>
> To: Maxime Ripard <[email protected]>
> To: Thomas Zimmermann <[email protected]>
> To: David Airlie <[email protected]>
> To: Simona Vetter <[email protected]>
> To: Ben Skeggs <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index dcc92131488e..a87406d3da30 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -304,6 +304,19 @@ nouveau_svmm_fini(struct nouveau_svmm **psvmm)
> {
> struct nouveau_svmm *svmm = *psvmm;
> if (svmm) {
> + struct nouveau_svm *svm = svmm->vmm->cli->drm->svm;
> +
> + /* The fault handler caches svmm pointers under svm-
> >mutex and
> + * dereferences them after dropping it, across
> blocking faults.
> + * The instance is already unlinked
> (nouveau_svmm_part), so drain
> + * the handler before the free to release any in-
> flight reference.
> + *
> + * On device teardown nouveau_svm_fini() ran first
> and freed
> + * drm->svm (NULL) after draining the buffer;
> nothing to flush.
> + */
> + if (svm)
> + flush_work(&svm->buffer[0].work);
> +
> mutex_lock(&svmm->mutex);
> svmm->vmm = NULL;
> mutex_unlock(&svmm->mutex);
>
> ---
> base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
> change-id: 20260813-nouveau-svm-svmm-uaf-bbcf36265aaa
>
> Best regards,
> --
> Zhenhao Wan <[email protected]>
