Re: [PATCH] drm/radeon: fix double free

[Alex Deucher]

Applied.  Thanks!

Alex

On Mon, Jul 6, 2020 at 8:29 AM <trix@xxxxxxxxxx> wrote:
>
> From: Tom Rix <trix@xxxxxxxxxx>
>
> clang static analysis flags this error
>
> drivers/gpu/drm/radeon/ci_dpm.c:5652:9: warning: Use of memory after it is freed [unix.Malloc]
>                 kfree(rdev->pm.dpm.ps[i].ps_priv);
>                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/radeon/ci_dpm.c:5654:2: warning: Attempt to free released memory [unix.Malloc]
>         kfree(rdev->pm.dpm.ps);
>         ^~~~~~~~~~~~~~~~~~~~~~
>
> problem is reported in ci_dpm_fini, with these code blocks.
>
>         for (i = 0; i < rdev->pm.dpm.num_ps; i++) {
>                 kfree(rdev->pm.dpm.ps[i].ps_priv);
>         }
>         kfree(rdev->pm.dpm.ps);
>
> The first free happens in ci_parse_power_table where it cleans up locally
> on a failure.  ci_dpm_fini also does a cleanup.
>
>         ret = ci_parse_power_table(rdev);
>         if (ret) {
>                 ci_dpm_fini(rdev);
>                 return ret;
>         }
>
> So remove the cleanup in ci_parse_power_table and
> move the num_ps calculation to inside the loop so ci_dpm_fini
> will know how many array elements to free.
>
> Fixes: cc8dbbb4f62a ("drm/radeon: add dpm support for CI dGPUs (v2)")
>
> Signed-off-by: Tom Rix <trix@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/radeon/ci_dpm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ci_dpm.c b/drivers/gpu/drm/radeon/ci_dpm.c
> index 86ac032275bb..ba20c6f03719 100644
> --- a/drivers/gpu/drm/radeon/ci_dpm.c
> +++ b/drivers/gpu/drm/radeon/ci_dpm.c
> @@ -5563,6 +5563,7 @@ static int ci_parse_power_table(struct radeon_device *rdev)
>         if (!rdev->pm.dpm.ps)
>                 return -ENOMEM;
>         power_state_offset = (u8 *)state_array->states;
> +       rdev->pm.dpm.num_ps = 0;
>         for (i = 0; i < state_array->ucNumEntries; i++) {
>                 u8 *idx;
>                 power_state = (union pplib_power_state *)power_state_offset;
> @@ -5572,10 +5573,8 @@ static int ci_parse_power_table(struct radeon_device *rdev)
>                 if (!rdev->pm.power_state[i].clock_info)
>                         return -EINVAL;
>                 ps = kzalloc(sizeof(struct ci_ps), GFP_KERNEL);
> -               if (ps == NULL) {
> -                       kfree(rdev->pm.dpm.ps);
> +               if (ps == NULL)
>                         return -ENOMEM;
> -               }
>                 rdev->pm.dpm.ps[i].ps_priv = ps;
>                 ci_parse_pplib_non_clock_info(rdev, &rdev->pm.dpm.ps[i],
>                                               non_clock_info,
> @@ -5597,8 +5596,8 @@ static int ci_parse_power_table(struct radeon_device *rdev)
>                         k++;
>                 }
>                 power_state_offset += 2 + power_state->v2.ucNumDPMLevels;
> +               rdev->pm.dpm.num_ps = i + 1;
>         }
> -       rdev->pm.dpm.num_ps = state_array->ucNumEntries;
>
>         /* fill in the vce power states */
>         for (i = 0; i < RADEON_MAX_VCE_LEVELS; i++) {
> --
> 2.18.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel