Am 16.06.2016 um 09:54 schrieb Dan Carpenter:
On Thu, Jun 16, 2016 at 09:26:03AM +0200, walter harms wrote:
Am 16.06.2016 08:41, schrieb Dan Carpenter:
There is no limit on high "idx" can go. It should be less than
ARRAY_SIZE(data.states) which is 16.
The "data" variable wasn't declared in that scope so I shifted the code
around a bit to make it work.
Fixes: f3898ea12fc1 ('drm/amd/powerplay: add some sysfs interfaces for powerplay.')
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 589b36e..ce9e97f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -275,25 +275,23 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
if (strlen(buf) == 1)
adev->pp_force_state_enabled = false;
- else {
- ret = kstrtol(buf, 0, &idx);
+ else if (adev->pp_enabled) {
+ struct pp_states_info data;
- if (ret) {
+ ret = kstrtol(buf, 0, &idx);
+ if (ret || idx >= ARRAY_SIZE(data.states)) {
count = -EINVAL;
goto fail;
}
i would also expect a check idx < 0, does it mean this can not happen ?
otherwise maybe kstrtoul is a solution ?
The original code could underflow, but my code can't. ARRAY_SIZE()
means the comparison is type promoted to size_t which is unsigned long.
That's probably true, but not very obvious (not that I understand much
of the power related code anyway).
Using kstrtoul() in the first place would make it a bit less obscure and
probably generate a nice error code when somebody really tries to use a
negative index here.
Cheers,
Christian.
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
