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 at oracle.com>
>
> 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 ?
re,
wh
>
> - if (adev->pp_enabled) {
> - struct pp_states_info data;
> - amdgpu_dpm_get_pp_num_states(adev, &data);
> - state = data.states[idx];
> - /* only set user selected power states */
> - if (state != POWER_STATE_TYPE_INTERNAL_BOOT &&
> - state != POWER_STATE_TYPE_DEFAULT) {
> - amdgpu_dpm_dispatch_task(adev,
> - AMD_PP_EVENT_ENABLE_USER_STATE,
> &state, NULL);
> - adev->pp_force_state_enabled = true;
> - }
> + amdgpu_dpm_get_pp_num_states(adev, &data);
> + state = data.states[idx];
> + /* only set user selected power states */
> + if (state != POWER_STATE_TYPE_INTERNAL_BOOT &&
> + state != POWER_STATE_TYPE_DEFAULT) {
> + amdgpu_dpm_dispatch_task(adev,
> + AMD_PP_EVENT_ENABLE_USER_STATE, &state,
> NULL);
> + adev->pp_force_state_enabled = true;
> }
> }
> fail:
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
