Re: [patch] drm/amdgpu: missing bounds check in amdgpu_set_pp_force_state()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux