Hans Verkuil <[email protected]> writes:

> The bo->resv pointer could be NULL, leading to kernel oopses like the one 
> below.
> It looks like .fb_probe -> drm_fbdev_cma_create() -> drm_gem_cma_create() 
> would
> end up not initializing resv, because we're doing that in vc4_bo_create(), not
> vc4_create_object().
>
> This patch ensures that bo->resv is always set in vc4_create_object
> ensuring that it is never NULL.
>
> Thanks to Eric Anholt for pointing to the correct solution.

Folks that know dma-buf better: Will having two reservations around for
the lifetime of the BO in the case of CMA dma-buf imports be a bad
thing?  I'm assuming reservations are cheap, given that we're
unconditionally allocating them per BO for un-shared BOs already.

> Signed-off-by: Hans Verkuil <[email protected]>
> ---
>   drivers/gpu/drm/vc4/vc4_bo.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index 80b2f9e55c5c..2231ee76cd8d 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -91,8 +91,7 @@ static void vc4_bo_destroy(struct vc4_bo *bo)
>       vc4->bo_stats.num_allocated--;
>       vc4->bo_stats.size_allocated -= obj->size;
>
> -     if (bo->resv == &bo->_resv)
> -             reservation_object_fini(bo->resv);
> +     reservation_object_fini(bo->resv);

This one would need to be "reservation_object_fini(&bo->_resv);" -- we
need to free the reservation that we created (and left unused in the
case of an import), not the one that was imported from someone else.

>       drm_gem_cma_free_object(obj);
>   }
> @@ -212,6 +211,8 @@ struct drm_gem_object *vc4_create_object(struct 
> drm_device *dev, size_t size)
>       vc4->bo_stats.num_allocated++;
>       vc4->bo_stats.size_allocated += size;
>       mutex_unlock(&vc4->bo_lock);
> +     bo->resv = &bo->_resv;
> +     reservation_object_init(bo->resv);
>
>       return &bo->base.base;
>   }
> @@ -250,12 +251,7 @@ struct vc4_bo *vc4_bo_create(struct drm_device *dev, 
> size_t unaligned_size,
>                       return ERR_PTR(-ENOMEM);
>               }
>       }
> -     bo = to_vc4_bo(&cma_obj->base);
> -
> -     bo->resv = &bo->_resv;
> -     reservation_object_init(bo->resv);
> -
> -     return bo;
> +     return to_vc4_bo(&cma_obj->base);
>   }
>
>   int vc4_dumb_create(struct drm_file *file_priv,
> -- 
> 2.11.0

Attachment: signature.asc
Description: PGP signature

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to