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

Open Source and information security mailing list archives

Date:   Thu, 6 Oct 2016 19:04:47 +0000
From:   "Deucher, Alexander" <Alexander.Deucher@....com>
To:     'Colin Ian King' <colin.king@...onical.com>,
        Alex Deucher <alexdeucher@...il.com>
CC:     "Koenig, Christian" <Christian.Koenig@....com>,
        David Airlie <airlied@...ux.ie>,
        "Huang, JinHuiEric" <JinHuiEric.Huang@....com>,
        "Zhu, Rex" <Rex.Zhu@....com>, "Zhou, Jammy" <Jammy.Zhou@....com>,
        "StDenis, Tom" <Tom.StDenis@....com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        "Maling list - DRI developers" <dri-devel@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] drm/amd/amdgpu: default to zero number of states if not
 enabled

> -----Original Message-----
> From: Colin Ian King [mailto:colin.king@...onical.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@...onical.com>
> wrote:
> >> From: Colin Ian King <colin.king@...onical.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@...onical.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@...ts.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives