From: "Deucher, Alexander" <Alexander.Deucher@amd.com>
To: "'Colin Ian King'" <colin.king@canonical.com>,
        Alex Deucher <alexdeucher@gmail.com>
CC: "Koenig, Christian" <Christian.Koenig@amd.com>,
        David Airlie <airlied@linux.ie>,
        "Huang, JinHuiEric" <JinHuiEric.Huang@amd.com>,
        "Zhu, Rex" <Rex.Zhu@amd.com>, "Zhou, Jammy" <Jammy.Zhou@amd.com>,
        "StDenis, Tom" <Tom.StDenis@amd.com>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        "Maling list - DRI developers" <dri-devel@lists.freedesktop.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] drm/amd/amdgpu: default to zero number of states if not
 enabled
Thread-Topic: [PATCH] drm/amd/amdgpu: default to zero number of states if not
 enabled
Thread-Index: AQHSH/vK2JqgrMDTT0anUGT7eZxl9qCbwCIAgAAIrYCAAAAmIA==
Date: Thu, 6 Oct 2016 19:04:47 +0000
Message-ID: <MWHPR12MB1694B591F5E428FEB8F89EBCF7C70@MWHPR12MB1694.namprd12.prod.outlook.com>
References: <20161006180211.31747-1-colin.king@canonical.com>
 <CADnq5_NUGhR8SS=Am6O+wcSBpZAvxq9bXGe7dzsosnxE_QLjEQ@mail.gmail.com>
 <57F6A01D.10907@canonical.com>
In-Reply-To: <57F6A01D.10907@canonical.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Oct 2016 19:04:47.2423
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1453
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u96J4vRH021043
Content-Length: 2224
Lines: 69

> -----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