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++)
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);
elsedata.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
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);
elsedata.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
-----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);
elsedata.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
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);
elsedata.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@lists.freedesktop.org
-
Alex Deucher -
Colin Ian King -
Colin King
-
Deucher, Alexander