drm/msm: Fix possible null dereference on failure of get_pages()
authorBen Hutchings <ben.hutchings@codethink.co.uk>
Tue, 3 Apr 2018 22:38:45 +0000 (23:38 +0100)
committerRob Clark <robdclark@gmail.com>
Sun, 3 Jun 2018 20:48:59 +0000 (16:48 -0400)
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.)

Fixes: 62e3a3e342af ("drm/msm: fix leak in failed get_pages")
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
drivers/gpu/drm/msm/msm_gem.c

index 95196479f651b18697229765432a0493f6b7d4ce..f583bb4222f9ad13dc17d74ec13d65ad16734506 100644 (file)
@@ -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);