From: Ben Hutchings ben.hutchings@codethink.co.uk
commit 3976626ea3d2011f8fd3f3a47070a8b792018253 upstream.
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 Cc: Guenter Roeck linux@roeck-us.net Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/gpu/drm/msm/msm_gem.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
--- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -116,17 +116,19 @@ static void put_pages(struct drm_gem_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);