* [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled @ 2016-10-06 18:02 ` Colin King 0 siblings, 0 replies; 9+ messages in thread From: Colin King @ 2016-10-06 18:02 UTC (permalink / raw) To: Alex Deucher, Christian König, David Airlie, Eric Huang, Rex Zhu, Jammy Zhou, Tom St Denis, Dan Carpenter, dri-devel Cc: linux-kernel 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. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled @ 2016-10-06 18:02 ` Colin King 0 siblings, 0 replies; 9+ messages in thread From: Colin King @ 2016-10-06 18:02 UTC (permalink / raw) To: Alex Deucher, Christian König, David Airlie, Eric Huang, Rex Zhu, Jammy Zhou, Tom St Denis, Dan Carpenter, dri-devel Cc: linux-kernel 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. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled 2016-10-06 18:02 ` Colin King @ 2016-10-06 18:32 ` Alex Deucher -1 siblings, 0 replies; 9+ messages in thread From: Alex Deucher @ 2016-10-06 18:32 UTC (permalink / raw) To: Colin King Cc: Tom St Denis, Jammy Zhou, LKML, Maling list - DRI developers, Eric Huang, Alex Deucher, Rex Zhu, Christian König, Dan Carpenter 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? The pp_num_states attribute only gets added in the first place if pp_enabled is true. 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled @ 2016-10-06 18:32 ` Alex Deucher 0 siblings, 0 replies; 9+ messages in thread From: Alex Deucher @ 2016-10-06 18:32 UTC (permalink / raw) To: Colin King Cc: Alex Deucher, Christian König, David Airlie, Eric Huang, Rex Zhu, Jammy Zhou, Tom St Denis, Dan Carpenter, Maling list - DRI developers, LKML 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? The pp_num_states attribute only gets added in the first place if pp_enabled is true. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled 2016-10-06 18:32 ` Alex Deucher @ 2016-10-06 19:03 ` Colin Ian King -1 siblings, 0 replies; 9+ messages in thread From: Colin Ian King @ 2016-10-06 19:03 UTC (permalink / raw) To: Alex Deucher Cc: Tom St Denis, Jammy Zhou, LKML, Maling list - DRI developers, Eric Huang, Alex Deucher, Rex Zhu, Christian König, Dan Carpenter 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? > > 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled @ 2016-10-06 19:03 ` Colin Ian King 0 siblings, 0 replies; 9+ messages in thread From: Colin Ian King @ 2016-10-06 19:03 UTC (permalink / raw) To: Alex Deucher Cc: Alex Deucher, Christian König, David Airlie, Eric Huang, Rex Zhu, Jammy Zhou, Tom St Denis, Dan Carpenter, Maling list - DRI developers, LKML 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? > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled 2016-10-06 19:03 ` Colin Ian King (?) @ 2016-10-06 19:04 ` Deucher, Alexander 2016-10-06 19:05 ` Colin Ian King -1 siblings, 1 reply; 9+ messages in thread From: Deucher, Alexander @ 2016-10-06 19:04 UTC (permalink / raw) To: 'Colin Ian King', Alex Deucher Cc: Koenig, Christian, David Airlie, Huang, JinHuiEric, Zhu, Rex, Zhou, Jammy, StDenis, Tom, Dan Carpenter, Maling list - DRI developers, LKML > -----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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled 2016-10-06 19:04 ` Deucher, Alexander @ 2016-10-06 19:05 ` Colin Ian King 0 siblings, 0 replies; 9+ messages in thread From: Colin Ian King @ 2016-10-06 19:05 UTC (permalink / raw) To: Deucher, Alexander, Alex Deucher Cc: StDenis, Tom, Zhou, Jammy, LKML, Maling list - DRI developers, Huang, JinHuiEric, Zhu, Rex, Koenig, Christian, Dan Carpenter On 06/10/16 20:04, Deucher, Alexander wrote: >> -----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. OK, in which case it's probably extraneous code that could be removed. > > 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 > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled @ 2016-10-06 19:05 ` Colin Ian King 0 siblings, 0 replies; 9+ messages in thread From: Colin Ian King @ 2016-10-06 19:05 UTC (permalink / raw) To: Deucher, Alexander, Alex Deucher Cc: Koenig, Christian, David Airlie, Huang, JinHuiEric, Zhu, Rex, Zhou, Jammy, StDenis, Tom, Dan Carpenter, Maling list - DRI developers, LKML On 06/10/16 20:04, Deucher, Alexander wrote: >> -----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. OK, in which case it's probably extraneous code that could be removed. > > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-10-06 19:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-06 18:02 [PATCH] drm/amd/amdgpu: default to zero number of states if not enabled Colin King 2016-10-06 18:02 ` Colin King 2016-10-06 18:32 ` Alex Deucher 2016-10-06 18:32 ` Alex Deucher 2016-10-06 19:03 ` Colin Ian King 2016-10-06 19:03 ` Colin Ian King 2016-10-06 19:04 ` Deucher, Alexander 2016-10-06 19:05 ` Colin Ian King 2016-10-06 19:05 ` Colin Ian King
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.