From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Thu, 16 Jun 2016 07:54:10 +0000 Subject: Re: [patch] drm/amdgpu: missing bounds check in amdgpu_set_pp_force_state() Message-Id: <20160616075410.GH32247@mwanda> List-Id: References: <20160616064119.GA23129@mwanda> <5762548B.6060906@bfs.de> In-Reply-To: <5762548B.6060906@bfs.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: walter harms Cc: Jammy Zhou , kernel-janitors@vger.kernel.org, dri-devel@lists.freedesktop.org, Eric Huang , Alex Deucher , Rex Zhu , Christian =?iso-8859-1?Q?K=F6nig?= 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 > > > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: [patch] drm/amdgpu: missing bounds check in amdgpu_set_pp_force_state() Date: Thu, 16 Jun 2016 10:54:10 +0300 Message-ID: <20160616075410.GH32247@mwanda> References: <20160616064119.GA23129@mwanda> <5762548B.6060906@bfs.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by gabe.freedesktop.org (Postfix) with ESMTPS id 112CE6E245 for ; Thu, 16 Jun 2016 07:54:39 +0000 (UTC) Content-Disposition: inline In-Reply-To: <5762548B.6060906@bfs.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: walter harms Cc: Jammy Zhou , kernel-janitors@vger.kernel.org, dri-devel@lists.freedesktop.org, Eric Huang , Alex Deucher , Rex Zhu , Christian =?iso-8859-1?Q?K=F6nig?= List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBKdW4gMTYsIDIwMTYgYXQgMDk6MjY6MDNBTSArMDIwMCwgd2FsdGVyIGhhcm1zIHdy b3RlOgo+IAo+IAo+IEFtIDE2LjA2LjIwMTYgMDg6NDEsIHNjaHJpZWIgRGFuIENhcnBlbnRlcjoK PiA+IFRoZXJlIGlzIG5vIGxpbWl0IG9uIGhpZ2ggImlkeCIgY2FuIGdvLiAgSXQgc2hvdWxkIGJl IGxlc3MgdGhhbgo+ID4gQVJSQVlfU0laRShkYXRhLnN0YXRlcykgd2hpY2ggaXMgMTYuCj4gPiAK PiA+IFRoZSAiZGF0YSIgdmFyaWFibGUgd2Fzbid0IGRlY2xhcmVkIGluIHRoYXQgc2NvcGUgc28g SSBzaGlmdGVkIHRoZSBjb2RlCj4gPiBhcm91bmQgYSBiaXQgdG8gbWFrZSBpdCB3b3JrLgo+ID4g Cj4gPiBGaXhlczogZjM4OThlYTEyZmMxICgnZHJtL2FtZC9wb3dlcnBsYXk6IGFkZCBzb21lIHN5 c2ZzIGludGVyZmFjZXMgZm9yIHBvd2VycGxheS4nKQo+ID4gU2lnbmVkLW9mZi1ieTogRGFuIENh cnBlbnRlciA8ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPgo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9ncHUvZHJtL2FtZC9hbWRncHUvYW1kZ3B1X3BtLmMgYi9kcml2ZXJzL2dwdS9kcm0v YW1kL2FtZGdwdS9hbWRncHVfcG0uYwo+ID4gaW5kZXggNTg5YjM2ZS4uY2U5ZTk3ZiAxMDA2NDQK PiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9hbWQvYW1kZ3B1L2FtZGdwdV9wbS5jCj4gPiArKysg Yi9kcml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfcG0uYwo+ID4gQEAgLTI3NSwyNSAr Mjc1LDIzIEBAIHN0YXRpYyBzc2l6ZV90IGFtZGdwdV9zZXRfcHBfZm9yY2Vfc3RhdGUoc3RydWN0 IGRldmljZSAqZGV2LAo+ID4gIAo+ID4gIAlpZiAoc3RybGVuKGJ1ZikgPT0gMSkKPiA+ICAJCWFk ZXYtPnBwX2ZvcmNlX3N0YXRlX2VuYWJsZWQgPSBmYWxzZTsKPiA+IC0JZWxzZSB7Cj4gPiAtCQly ZXQgPSBrc3RydG9sKGJ1ZiwgMCwgJmlkeCk7Cj4gPiArCWVsc2UgaWYgKGFkZXYtPnBwX2VuYWJs ZWQpIHsKPiA+ICsJCXN0cnVjdCBwcF9zdGF0ZXNfaW5mbyBkYXRhOwo+ID4gIAo+ID4gLQkJaWYg KHJldCkgewo+ID4gKwkJcmV0ID0ga3N0cnRvbChidWYsIDAsICZpZHgpOwo+ID4gKwkJaWYgKHJl dCB8fCBpZHggPj0gQVJSQVlfU0laRShkYXRhLnN0YXRlcykpIHsKPiA+ICAJCQljb3VudCA9IC1F SU5WQUw7Cj4gPiAgCQkJZ290byBmYWlsOwo+ID4gIAkJfQo+IAo+IAo+IGkgd291bGQgYWxzbyBl eHBlY3QgYSBjaGVjayBpZHggPCAwLCBkb2VzIGl0IG1lYW4gdGhpcyBjYW4gbm90IGhhcHBlbiA/ Cj4gb3RoZXJ3aXNlIG1heWJlIGtzdHJ0b3VsIGlzIGEgc29sdXRpb24gPwoKVGhlIG9yaWdpbmFs IGNvZGUgY291bGQgdW5kZXJmbG93LCBidXQgbXkgY29kZSBjYW4ndC4gIEFSUkFZX1NJWkUoKQpt ZWFucyB0aGUgY29tcGFyaXNvbiBpcyB0eXBlIHByb21vdGVkIHRvIHNpemVfdCB3aGljaCBpcyB1 bnNpZ25lZCBsb25nLgoKcmVnYXJkcywKZGFuIGNhcnBlbnRlcgoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmkt ZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==