Re: [RFC PATCH 1/3] drm/virtio: .release ops for virtgpu fence release

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 8/17/23 08:25, Kim, Dongwon wrote:
...
> Yeah, I know it frees 'struct dma_fence *f' but what about 'struct
> virtio_gpu_fence *fence'? This is a device specific fence that contains
> struct dma_fence *f. But hold on... so when fence->ops->release is
> called then dma_fence_free won't be called here:
> 
>     if (fence->ops->release)
>         fence->ops->release(fence);
>     else
>         dma_fence_free(fence);
> 
> In that case, I think virtio_gpu_fence_release should do
> "dma_fence_free(f)" before freeing virtio_gpu_fence? Am I right?
> Like,
> 
> static void virtio_gpu_fence_release(struct dma_fence *f)
> {
>     struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f);
> 
>     dma_fence_free(f);
>     kfree(fence);
> }

That is a double free and wrong of course. Both dma_fence *f and
virtio_gpu_fence *fence point at the same kmemory object. See
to_virtio_gpu_fence() and please research how container_of() works.

-- 
Best regards,
Dmitry