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 <jcrouse@xxxxxxxxxxxxxx>
> Fixes: 62e3a3e342af ("drm/msm: fix leak in failed get_pages")
> Signed-off-by: Ben Hutchings <ben.hutchings@xxxxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
