[PATCH] drm/amd/display: Use after free in dmub_psr_destroy()

[Dan Carpenter]

On Fri, Feb 28, 2020 at 11:05:11AM +0100, Michel Dänzer wrote:
> On 2020-02-28 9:22 a.m., Dan Carpenter wrote:
> > These lines need to be re-ordered so that we don't dereference "dmub"
> > after we just freed it.
> > 
> > Fixes: 4c1a1335dfe0 ("drm/amd/display: Driverside changes to support PSR in DMCUB")
> > Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> > ---
> >  drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> > index 2c932c29f1f9..dc858b152c6e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
> > @@ -235,6 +235,6 @@ struct dmub_psr *dmub_psr_create(struct dc_context *ctx)
> >   */
> >  void dmub_psr_destroy(struct dmub_psr **dmub)
> >  {
> > -	kfree(dmub);
> >  	*dmub = NULL;
> > +	kfree(dmub);
> >  }
> > 
> 
> Maybe
> 
> 	kfree(*dmub);
> 
> was intended instead?
> 

Ah yeah.  You're right.  I will resend.

> 
> Actually, this function and others in this file seem completely unused?

It's used in linux-next from dcn21_resource_destruct().

drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
   986  
   987          if (pool->base.dp_clock_source != NULL) {
   988                  dcn20_clock_source_destroy(&pool->base.dp_clock_source);
   989                  pool->base.dp_clock_source = NULL;
   990          }
   991  
   992  
   993          if (pool->base.abm != NULL)
   994                  dce_abm_destroy(&pool->base.abm);
   995  
   996          if (pool->base.dmcu != NULL)
   997                  dce_dmcu_destroy(&pool->base.dmcu);
   998  
   999          if (pool->base.psr != NULL)
  1000                  dmub_psr_destroy(&pool->base.psr);

kfree(&pool->base.psr); will crash.

  1001  
  1002          if (pool->base.dccg != NULL)
  1003                  dcn_dccg_destroy(&pool->base.dccg);
  1004  
  1005          if (pool->base.pp_smu != NULL)
  1006                  dcn21_pp_smu_destroy(&pool->base.pp_smu);
  1007  }


regards,
dan carpenter