On Tue, Apr 03, 2018 at 11:38:45PM +0100, Ben Hutchings wrote:
> Commit 62e3a3e342af changed get_pages() to initialise
> msm_gem_object::pages before trying to initialise msm_gem_object::sgt,
> so that put_pages() would properly clean up pages in the failure
> case.
> 
> However, this means that put_pages() now needs to check that
> msm_gem_object::sgt is not null before trying to clean it up, and
> this check was only applied to part of the cleanup code.  Move
> it all into the conditional block.  (Strictly speaking we don't
> need to make the kfree() conditional, but since we can't avoid
> checking for null ourselves we may as well do so.)

Seems legit to me. Thanks for the catch.

Reviewed-by: Jordan Crouse <jcro...@codeaurora.org>

> Fixes: 62e3a3e342af ("drm/msm: fix leak in failed get_pages")
> Signed-off-by: Ben Hutchings <ben.hutchi...@codethink.co.uk>
> ---
>  drivers/gpu/drm/msm/msm_gem.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 07376de9ff4c..37ec3411297b 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -132,17 +132,19 @@ static void put_pages(struct drm_gem_object *obj)
>       struct msm_gem_object *msm_obj = to_msm_bo(obj);
>  
>       if (msm_obj->pages) {
> -             /* For non-cached buffers, ensure the new pages are clean
> -              * because display controller, GPU, etc. are not coherent:
> -              */
> -             if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
> -                     dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl,
> -                                     msm_obj->sgt->nents, DMA_BIDIRECTIONAL);
> +             if (msm_obj->sgt) {
> +                     /* For non-cached buffers, ensure the new
> +                      * pages are clean because display controller,
> +                      * GPU, etc. are not coherent:
> +                      */
> +                     if (msm_obj->flags & (MSM_BO_WC|MSM_BO_UNCACHED))
> +                             dma_unmap_sg(obj->dev->dev, msm_obj->sgt->sgl,
> +                                          msm_obj->sgt->nents,
> +                                          DMA_BIDIRECTIONAL);
>  
> -             if (msm_obj->sgt)
>                       sg_free_table(msm_obj->sgt);
> -
> -             kfree(msm_obj->sgt);
> +                     kfree(msm_obj->sgt);
> +             }
>  
>               if (use_pages(obj))
>                       drm_gem_put_pages(obj, msm_obj->pages, true, false);
> -- 
> 2.16.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to