This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..ee4a8e44fbeb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1931,7 +1931,7 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, uint32_t mask) { int ret = 0; - uint32_t i = 0; + int i;
for (i = 0; i < counts; i++) { ret = amdgpu_device_attr_create(adev, &attrs[i], mask); @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed: - for (; i > 0; i--) { + while (--i >= 0) amdgpu_device_attr_remove(adev, &attrs[i]); - }
return ret; }
Am 20.05.20 um 14:00 schrieb Dan Carpenter:
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..ee4a8e44fbeb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1931,7 +1931,7 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, uint32_t mask) { int ret = 0;
- uint32_t i = 0;
int i;
for (i = 0; i < counts; i++) { ret = amdgpu_device_attr_create(adev, &attrs[i], mask);
@@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed:
- for (; i > 0; i--) {
- while (--i >= 0)
As far as I know the common idiom for this is while (i--) which even works without changing the type of i to signed.
Christian.
amdgpu_device_attr_remove(adev, &attrs[i]);
}
return ret; }
On Wed, May 20, 2020 at 02:05:19PM +0200, Christian König wrote:
Am 20.05.20 um 14:00 schrieb Dan Carpenter:
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..ee4a8e44fbeb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1931,7 +1931,7 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, uint32_t mask) { int ret = 0;
- uint32_t i = 0;
- int i; for (i = 0; i < counts; i++) { ret = amdgpu_device_attr_create(adev, &attrs[i], mask);
@@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0; failed:
- for (; i > 0; i--) {
- while (--i >= 0)
As far as I know the common idiom for this is while (i--) which even works without changing the type of i to signed.
It's about 50/50, one way or the other. To me --i >= 0 seems far more readable.
I've been trying to figure out which tool tells people to make iterators unsigned so I can help them avoid it. :/ I understand how in theory iterators could go above INT_MAX but if we're going above INT_MAX then probably we should use a 64 bit type. There are very few times where 2 billion iterations is not enough but in those situations probably 4 billion is not enough either. So unsigned int iterators never or seldom solve real life bugs but they regularly cause them.
regards, dan carpenter
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- v2: style change
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed: - for (; i > 0; i--) { + while (i--) amdgpu_device_attr_remove(adev, &attrs[i]); - }
return ret; }
[AMD Official Use Only - Internal Distribution Only]
thanks.
Reviewed-by: Kevin Wang kevin1.wang@amd.com
Best Regads, Kevin ________________________________ From: Dan Carpenter dan.carpenter@oracle.com Sent: Wednesday, May 20, 2020 9:08 PM To: Deucher, Alexander Alexander.Deucher@amd.com; Wang, Kevin(Yang) Kevin1.Wang@amd.com Cc: Koenig, Christian Christian.Koenig@amd.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; Quan, Evan Evan.Quan@amd.com; Huang, Ray Ray.Huang@amd.com; Feng, Kenneth Kenneth.Feng@amd.com; Tao, Yintian Yintian.Tao@amd.com; Zhang, Hawking Hawking.Zhang@amd.com; amd-gfx@lists.freedesktop.org amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org kernel-janitors@vger.kernel.org Subject: [PATCH v2] drm/amdgpu: off by on in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- v2: style change
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed: - for (; i > 0; i--) { + while (i--) amdgpu_device_attr_remove(adev, &attrs[i]); - }
return ret; }
"off by on"
or
"off by one"
?
M
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Dan Carpenter Sent: Wednesday, May 20, 2020 9:08 AM To: Alex Deucher alexander.deucher@amd.com; Kevin Wang kevin1.wang@amd.com Cc: David Airlie airlied@linux.ie; kernel-janitors@vger.kernel.org; linux- kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org; Hawking Zhang Hawking.Zhang@amd.com; Rui Huang ray.huang@amd.com; dri- devel@lists.freedesktop.org; Evan Quan evan.quan@amd.com; Kenneth Feng kenneth.feng@amd.com; Christian König christian.koenig@amd.com; Yintian Tao yttao@amd.com Subject: [PATCH v2] drm/amdgpu: off by on in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
v2: style change
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed:
- for (; i > 0; i--) {
- while (i--) amdgpu_device_attr_remove(adev, &attrs[i]);
}
return ret;
} _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com --- v2: style change v3: Fix embarrassing typo in the subject
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed: - for (; i > 0; i--) { + while (i--) amdgpu_device_attr_remove(adev, &attrs[i]); - }
return ret; }
-----Original Message----- From: Dan Carpenter dan.carpenter@oracle.com Sent: Wednesday, May 20, 2020 11:26 AM To: Alex Deucher alexander.deucher@amd.com; Kevin Wang kevin1.wang@amd.com; Ruhl, Michael J michael.j.ruhl@intel.com Cc: Christian König christian.koenig@amd.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; Evan Quan evan.quan@amd.com; Rui Huang ray.huang@amd.com; Kenneth Feng kenneth.feng@amd.com; Yintian Tao yttao@amd.com; Hawking Zhang Hawking.Zhang@amd.com; amd-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel- janitors@vger.kernel.org Subject: [PATCH v3] drm/amdgpu: off by one in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
v2: style change v3: Fix embarrassing typo in the subject
😊
Acked-by: Michael J. Ruhl michael.j.ruhl@intel.com
m
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed:
- for (; i > 0; i--) {
- while (i--) amdgpu_device_attr_remove(adev, &attrs[i]);
}
return ret;
}
Am 20.05.20 um 17:31 schrieb Ruhl, Michael J:
-----Original Message----- From: Dan Carpenter dan.carpenter@oracle.com Sent: Wednesday, May 20, 2020 11:26 AM To: Alex Deucher alexander.deucher@amd.com; Kevin Wang kevin1.wang@amd.com; Ruhl, Michael J michael.j.ruhl@intel.com Cc: Christian König christian.koenig@amd.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; Evan Quan evan.quan@amd.com; Rui Huang ray.huang@amd.com; Kenneth Feng kenneth.feng@amd.com; Yintian Tao yttao@amd.com; Hawking Zhang Hawking.Zhang@amd.com; amd-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel- janitors@vger.kernel.org Subject: [PATCH v3] drm/amdgpu: off by one in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute code") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
v2: style change v3: Fix embarrassing typo in the subject
😊
Acked-by: Michael J. Ruhl michael.j.ruhl@intel.com
Reviewed-by: Christian König christian.koenig@amd.com
m
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed:
- for (; i > 0; i--) {
- while (i--) amdgpu_device_attr_remove(adev, &attrs[i]);
}
return ret;
}
Applied. Thanks!
Alex
On Wed, May 20, 2020 at 11:33 AM Christian König christian.koenig@amd.com wrote:
Am 20.05.20 um 17:31 schrieb Ruhl, Michael J:
-----Original Message----- From: Dan Carpenter dan.carpenter@oracle.com Sent: Wednesday, May 20, 2020 11:26 AM To: Alex Deucher alexander.deucher@amd.com; Kevin Wang kevin1.wang@amd.com; Ruhl, Michael J michael.j.ruhl@intel.com Cc: Christian König christian.koenig@amd.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; Evan Quan evan.quan@amd.com; Rui Huang ray.huang@amd.com; Kenneth Feng kenneth.feng@amd.com; Yintian Tao yttao@amd.com; Hawking Zhang Hawking.Zhang@amd.com; amd-gfx@lists.freedesktop.org; dri- devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; kernel- janitors@vger.kernel.org Subject: [PATCH v3] drm/amdgpu: off by one in amdgpu_device_attr_create_groups() error handling
This loop in the error handling code should start a "i - 1" and end at "i == 0". Currently it starts a "i" and ends at "i == 1". The result is that it removes one attribute that wasn't created yet, and leaks the zeroeth attribute.
Fixes: 4e01847c38f7 ("drm/amdgpu: optimize amdgpu device attribute
code")
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
v2: style change v3: Fix embarrassing typo in the subject
😊
Acked-by: Michael J. Ruhl michael.j.ruhl@intel.com
Reviewed-by: Christian König christian.koenig@amd.com
m
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c index b75362bf0742..e809534fabd4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c @@ -1942,9 +1942,8 @@ static int amdgpu_device_attr_create_groups(struct amdgpu_device *adev, return 0;
failed:
- for (; i > 0; i--) {
- while (i--) amdgpu_device_attr_remove(adev, &attrs[i]);
}
return ret;
}
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König -
Christian König -
Dan Carpenter -
Ruhl, Michael J -
Wang, Kevin(Yang)