<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 2025-08-26 16:58, Alex Deucher wrote: </div> <blockquote type="cite" cite="mid:CADnq5_MCiwaLJEXG1y9-D0v8naz0ZnNVurs-cQXBDvAyL+oXDg@mail.gmail.com"> <pre wrap="" class="moz-quote-pre">On Tue, Aug 26, 2025 at 3:48 PM David (Ming Qiang) Wu <a class="moz-txt-link-rfc2396E" href="mailto:David.Wu3@amd.com"><David.Wu3@amd.com></a> wrote: </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre"> to_amdgpu_fence() could return NULL pointer which needs to be protected for dereferencing. </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> I don't think any of these cases could ever be NULL. The amdgpu_fence is a container for the dma_fence so it will alway be present. See struct amdgpu_fence.</pre> </blockquote> <pre>hmmm... but - the function could return NULL based on base.ops - see below if it should never happen then we should remove the checking. I doubt we will ever return NULL, however someone knowledgeable PLEASE fix it properly. The patch is only to protect it just in case. </pre> <pre>static inline struct amdgpu_fence *to_amdgpu_fence(struct dma_fence *f)</pre> <pre>{ struct amdgpu_fence *__f = container_of(f, struct amdgpu_fence, base); if (__f->base.ops == &amdgpu_fence_ops || __f->base.ops == &amdgpu_job_fence_ops) return __f; return NULL; } </pre> <blockquote type="cite" cite="mid:CADnq5_MCiwaLJEXG1y9-D0v8naz0ZnNVurs-cQXBDvAyL+oXDg@mail.gmail.com"> <pre wrap="" class="moz-quote-pre"> Alex </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre"> Signed-off-by: David (Ming Qiang) Wu <a class="moz-txt-link-rfc2396E" href="mailto:David.Wu3@amd.com"><David.Wu3@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c index 2d58aefbd68a7..1432b64d379ef 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c @@ -160,7 +160,9 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct dma_fence **f, } } - to_amdgpu_fence(fence)->start_timestamp = ktime_get(); + am_fence = to_amdgpu_fence(fence); + if (am_fence) + am_fence->start_timestamp = ktime_get(); /* This function can't be called concurrently anyway, otherwise * emitting the fence would mess up the hardware ring buffer. @@ -387,6 +389,7 @@ u64 amdgpu_fence_last_unsignaled_time_us(struct amdgpu_ring *ring) struct amdgpu_fence_driver *drv = &ring->fence_drv; struct dma_fence *fence; uint32_t last_seq, sync_seq; + struct amdgpu_fence *f; last_seq = atomic_read(&ring->fence_drv.last_seq); sync_seq = READ_ONCE(ring->fence_drv.sync_seq); @@ -399,8 +402,8 @@ u64 amdgpu_fence_last_unsignaled_time_us(struct amdgpu_ring *ring) if (!fence) return 0; - return ktime_us_delta(ktime_get(), - to_amdgpu_fence(fence)->start_timestamp); + f = to_amdgpu_fence(fence); + return f ? ktime_us_delta(ktime_get(), f->start_timestamp) : 0; } /** @@ -417,13 +420,16 @@ void amdgpu_fence_update_start_timestamp(struct amdgpu_ring *ring, uint32_t seq, { struct amdgpu_fence_driver *drv = &ring->fence_drv; struct dma_fence *fence; + struct amdgpu_fence *f; seq &= drv->num_fences_mask; fence = drv->fences[seq]; if (!fence) return; - to_amdgpu_fence(fence)->start_timestamp = timestamp; + f = to_amdgpu_fence(fence); + if (f) + f->start_timestamp = timestamp; } /** @@ -827,7 +833,8 @@ static const char *amdgpu_fence_get_driver_name(struct dma_fence *fence) static const char *amdgpu_fence_get_timeline_name(struct dma_fence *f) { - return (const char *)to_amdgpu_fence(f)->ring->name; + struct amdgpu_fence *am_f = to_amdgpu_fence(f); + return (const char *) (am_f ? am_f->ring->name : ""); } static const char *amdgpu_job_fence_get_timeline_name(struct dma_fence *f) @@ -847,8 +854,9 @@ static const char *amdgpu_job_fence_get_timeline_name(struct dma_fence *f) */ static bool amdgpu_fence_enable_signaling(struct dma_fence *f) { - if (!timer_pending(&to_amdgpu_fence(f)->ring->fence_drv.fallback_timer)) - amdgpu_fence_schedule_fallback(to_amdgpu_fence(f)->ring); + struct amdgpu_fence *am_f = to_amdgpu_fence(f); + if (am_f && !timer_pending(&am_f->ring->fence_drv.fallback_timer)) + amdgpu_fence_schedule_fallback(am_f->ring); return true; } -- 2.43.0 </pre> </blockquote> </blockquote> </body> </html>