[PATCH] drm/nouveau: Fix NULL pointer dereference in nouveau_fence_sync() when prev->cli == NULL

[PATCH] drm/nouveau: Fix NULL pointer dereference in nouveau_fence_sync() when prev->cli == NULL

[Emmanuel Fleury]

I ran several times into a kernel oops caused by a NULL pointer dereference
in nouveau_fence_sync(). The variable "prev" may be non-NULL while
"prev->cli" is NULL, leading to an unconditional dereference of
"prev->cli->drm".

Prevent the dereference by checking "prev->cli" before accessing
its "drm" field while preserving the existing logic.

Fixes: 1f9910b41c857 ("nouveau/fence: handle cross device fences properly")
Signed-off-by: Emmanuel Fleury <emmanuel.fleury@u-bordeaux.fr>
---
  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c 
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index edbe9e08ba0f..90e6e89c0911 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -374,7 +374,7 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct 
nouveau_channel *chan,

  				rcu_read_lock();
  				prev = rcu_dereference(f->channel);
-				local = prev && prev->cli->drm == chan->cli->drm;
+				local = prev && prev->cli && prev->cli->drm == chan->cli->drm;
  				if (local && (prev == chan ||
  					      fctx->sync(f, prev, chan) == 0))
  					must_wait = false;
-- 
2.53.0

[PATCH] drm/nouveau: Fix NULL pointer dereference in nouveau_fence_sync() when prev->cli == NULL

[Emmanuel Fleury]

I ran several times into a kernel oops caused by a NULL pointer dereference
in nouveau_fence_sync(). The variable "prev" may be non-NULL while
"prev->cli" is NULL, leading to an unconditional dereference of
"prev->cli->drm".

Prevent the dereference by checking "prev->cli" before accessing
its "drm" field while preserving the existing logic.

Fixes: 1f9910b41c857 ("nouveau/fence: handle cross device fences properly")
Signed-off-by: Emmanuel Fleury <emmanuel.fleury@u-bordeaux.fr>
---
  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c 
b/drivers/gpu/drm/nouveau/nouveau_fence.c
index edbe9e08ba0f..90e6e89c0911 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -374,7 +374,7 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct 
nouveau_channel *chan,

  				rcu_read_lock();
  				prev = rcu_dereference(f->channel);
-				local = prev && prev->cli->drm == chan->cli->drm;
+				local = prev && prev->cli && prev->cli->drm == chan->cli->drm;
  				if (local && (prev == chan ||
  					      fctx->sync(f, prev, chan) == 0))
  					must_wait = false;
-- 
2.53.0

Re: [PATCH] drm/nouveau: Fix NULL pointer dereference in nouveau_fence_sync() when prev->cli == NULL

[Beomseok Kim]

Hi Emmanuel,

I have been looking into what appears to be the same NULL dereference in
nouveau_fence_sync(), and I found a path that may explain how prev->cli
ends up NULL.

In the failure I traced, nouveau_fence_no_signaling() removes the fence
from the pending list, but leaves fence->channel pointing to the channel.
The fence itself can remain alive through a BO reservation. After the
channel is torn down, the same fence can later be encountered again in
nouveau_fence_sync() through that stale channel association.

The normal nouveau_fence_signal() path clears fence->channel when removing
the fence from the pending list, so the no-signaling path appears
asymmetric here.

I also did not find a normal path that explicitly sets channel->cli to
NULL while keeping the channel valid. If this is the path leading to the
crash, checking prev->cli here would prevent the dereference, but the stale
fence->channel association would still remain.

The existing fence/channel lifetime handling seems to rely on clearing
fence->channel once the fence is detached from the channel. If that is the
intended invariant, preserving it in the no-signaling path seems preferable
to allowing a non-NULL fence->channel to refer to a torn-down channel.

I sent a separate patch that clears fence->channel in
nouveau_fence_no_signaling():

  [PATCH] drm/nouveau: Clear fence channel in no-signaling path
  https://lore.kernel.org/r/20260928081450.19340-1-dilddream31@gmail.com

Thanks,
Beomseok

Re: [PATCH] drm/nouveau: Fix NULL pointer dereference in nouveau_fence_sync() when prev->cli == NULL

[Beomseok Kim]

Hi Emmanuel,

I have been looking into what appears to be the same NULL dereference in
nouveau_fence_sync(), and I found a path that may explain how prev->cli
ends up NULL.

In the failure I traced, nouveau_fence_no_signaling() removes the fence
from the pending list, but leaves fence->channel pointing to the channel.
The fence itself can remain alive through a BO reservation. After the
channel is torn down, the same fence can later be encountered again in
nouveau_fence_sync() through that stale channel association.

The normal nouveau_fence_signal() path clears fence->channel when removing
the fence from the pending list, so the no-signaling path appears
asymmetric here.

I also did not find a normal path that explicitly sets channel->cli to
NULL while keeping the channel valid. If this is the path leading to the
crash, checking prev->cli here would prevent the dereference, but the stale
fence->channel association would still remain.

The existing fence/channel lifetime handling seems to rely on clearing
fence->channel once the fence is detached from the channel. If that is the
intended invariant, preserving it in the no-signaling path seems preferable
to allowing a non-NULL fence->channel to refer to a torn-down channel.

I sent a separate patch that clears fence->channel in
nouveau_fence_no_signaling():

  [PATCH] drm/nouveau: Clear fence channel in no-signaling path
  https://lore.kernel.org/r/20260928081450.19340-1-dilddream31@gmail.com

Thanks,
Beomseok