Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1496820

RE: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled

Show all headers | View raw

> -----Original Message-----
> From: Colin Ian King [mailto:colin.king@canonical.com]
> Sent: Thursday, October 06, 2016 3:04 PM
> To: Alex Deucher
> Cc: Deucher, Alexander; Koenig, Christian; David Airlie; Huang, JinHuiEric;
> Zhu, Rex; Zhou, Jammy; StDenis, Tom; Dan Carpenter; Maling list - DRI
> developers; LKML
> Subject: Re: [PATCH] drm/amd/amdgpu: default to zero number of states if
> not enabled
> 
> On 06/10/16 19:32, Alex Deucher wrote:
> > On Thu, Oct 6, 2016 at 2:02 PM, Colin King <colin.king@canonical.com>
> wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> Currently, if adev->pp_enabled is false then the pp_stats_info data
> >> is not read and hence a garbage number of states from the stack
> >> is used to dump out the number of states. Given data.nums could be
> >> any random value, this could easily lead to read outside the
> >> data.states array.  Fix this by setting data.nums to zero if
> >> adev->pp_enabled is false.
> >
> > Are you actually seeing a problem?
> 
> Nope.
> 
> > The pp_num_states attribute only
> > gets added in the first place if pp_enabled is true.
> 
> Does that mean that the check on adev->pp_enabled is redundant then?

Yes, I think so.

Alex

> 
> >
> > Alex
> 
> >
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> >> index accc908..808d788 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> >> @@ -195,6 +195,8 @@ static ssize_t amdgpu_get_pp_num_states(struct
> device *dev,
> >>
> >>         if (adev->pp_enabled)
> >>                 amdgpu_dpm_get_pp_num_states(adev, &data);
> >> +       else
> >> +               data.nums = 0;
> >>
> >>         buf_len = snprintf(buf, PAGE_SIZE, "states: %d\n", data.nums);
> >>         for (i = 0; i < data.nums; i++)
> >> --
> >> 2.9.3
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH] drm/amd/amdgpu: default to zero number of states if not enabled Colin King <colin.king@canonical.com> - 2016-10-06 20:10 +0200
  Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled Alex Deucher <alexdeucher@gmail.com> - 2016-10-06 20:40 +0200
    Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not  enabled Colin Ian King <colin.king@canonical.com> - 2016-10-06 21:10 +0200
      Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not  enabled Colin Ian King <colin.king@canonical.com> - 2016-10-06 21:10 +0200
      RE: [PATCH] drm/amd/amdgpu: default to zero number of states if not  enabled "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2016-10-06 21:10 +0200

csiph-web