All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Fix realloc of ptr
@ 2022-02-26 15:58 ` trix
  0 siblings, 0 replies; 6+ messages in thread
From: trix @ 2022-02-26 15:58 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	nathan, ndesaulniers, nirmoy.das, lijo.lazar, tom.stdenis,
	evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: amd-gfx, dri-devel, linux-kernel, llvm, Tom Rix

From: Tom Rix <trix@redhat.com>

Clang static analysis reports this error
amdgpu_debugfs.c:1690:9: warning: 1st function call
  argument is an uninitialized value
  tmp = krealloc_array(tmp, i + 1,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~

realloc will free tmp, so tmp can not be garbage.
And the return needs to be checked.

Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 9eb9b440bd438..159b97c0b4ebc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
 	char reg_offset[11];
-	uint32_t *tmp;
+	uint32_t *tmp = NULL;
 	int ret, i = 0, len = 0;
 
 	do {
@@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
 		}
 
 		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
+		if (!tmp) {
+			ret = -ENOMEM;
+			goto error_free;
+		}
 		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
 			ret = -EINVAL;
 			goto error_free;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] drm/amdgpu: Fix realloc of ptr
@ 2022-02-26 15:58 ` trix
  0 siblings, 0 replies; 6+ messages in thread
From: trix @ 2022-02-26 15:58 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	nathan, ndesaulniers, nirmoy.das, lijo.lazar, tom.stdenis,
	evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: Tom Rix, llvm, dri-devel, amd-gfx, linux-kernel

From: Tom Rix <trix@redhat.com>

Clang static analysis reports this error
amdgpu_debugfs.c:1690:9: warning: 1st function call
  argument is an uninitialized value
  tmp = krealloc_array(tmp, i + 1,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~

realloc will free tmp, so tmp can not be garbage.
And the return needs to be checked.

Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 9eb9b440bd438..159b97c0b4ebc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
 	char reg_offset[11];
-	uint32_t *tmp;
+	uint32_t *tmp = NULL;
 	int ret, i = 0, len = 0;
 
 	do {
@@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
 		}
 
 		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
+		if (!tmp) {
+			ret = -ENOMEM;
+			goto error_free;
+		}
 		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
 			ret = -EINVAL;
 			goto error_free;
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: Fix realloc of ptr
  2022-02-26 15:58 ` trix
@ 2022-02-26 22:21   ` David Laight
  -1 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2022-02-26 22:21 UTC (permalink / raw)
  To: 'trix@redhat.com',
	alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	nathan, ndesaulniers, nirmoy.das, lijo.lazar, tom.stdenis,
	evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: amd-gfx, dri-devel, linux-kernel, llvm

From: trix@redhat.com
> Sent: 26 February 2022 15:59
> 
> From: Tom Rix <trix@redhat.com>
> 
> Clang static analysis reports this error
> amdgpu_debugfs.c:1690:9: warning: 1st function call
>   argument is an uninitialized value
>   tmp = krealloc_array(tmp, i + 1,
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> realloc will free tmp, so tmp can not be garbage.
> And the return needs to be checked.

Are you sure?
A quick check seems to show that krealloc() behaves the same
way as libc realloc() and the pointer isn't freed on failure.

	David

> Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
> Signed-off-by: Tom Rix <trix@redhat.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 9eb9b440bd438..159b97c0b4ebc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>  {
>  	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
>  	char reg_offset[11];
> -	uint32_t *tmp;
> +	uint32_t *tmp = NULL;
>  	int ret, i = 0, len = 0;
> 
>  	do {
> @@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>  		}
> 
>  		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
> +		if (!tmp) {
> +			ret = -ENOMEM;
> +			goto error_free;
> +		}
>  		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
>  			ret = -EINVAL;
>  			goto error_free;
> --
> 2.26.3

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu: Fix realloc of ptr
@ 2022-02-26 22:21   ` David Laight
  0 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2022-02-26 22:21 UTC (permalink / raw)
  To: 'trix@redhat.com',
	alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel,
	nathan, ndesaulniers, nirmoy.das, lijo.lazar, tom.stdenis,
	evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: llvm, dri-devel, amd-gfx, linux-kernel

From: trix@redhat.com
> Sent: 26 February 2022 15:59
> 
> From: Tom Rix <trix@redhat.com>
> 
> Clang static analysis reports this error
> amdgpu_debugfs.c:1690:9: warning: 1st function call
>   argument is an uninitialized value
>   tmp = krealloc_array(tmp, i + 1,
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> realloc will free tmp, so tmp can not be garbage.
> And the return needs to be checked.

Are you sure?
A quick check seems to show that krealloc() behaves the same
way as libc realloc() and the pointer isn't freed on failure.

	David

> Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
> Signed-off-by: Tom Rix <trix@redhat.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 9eb9b440bd438..159b97c0b4ebc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>  {
>  	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
>  	char reg_offset[11];
> -	uint32_t *tmp;
> +	uint32_t *tmp = NULL;
>  	int ret, i = 0, len = 0;
> 
>  	do {
> @@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>  		}
> 
>  		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
> +		if (!tmp) {
> +			ret = -ENOMEM;
> +			goto error_free;
> +		}
>  		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
>  			ret = -EINVAL;
>  			goto error_free;
> --
> 2.26.3

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Fix realloc of ptr
  2022-02-26 22:21   ` David Laight
@ 2022-02-27 14:36     ` Tom Rix
  -1 siblings, 0 replies; 6+ messages in thread
From: Tom Rix @ 2022-02-27 14:36 UTC (permalink / raw)
  To: David Laight, alexander.deucher, christian.koenig, Xinhui.Pan,
	airlied, daniel, nathan, ndesaulniers, nirmoy.das, lijo.lazar,
	tom.stdenis, evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: amd-gfx, dri-devel, linux-kernel, llvm


On 2/26/22 2:21 PM, David Laight wrote:
> From: trix@redhat.com
>> Sent: 26 February 2022 15:59
>>
>> From: Tom Rix <trix@redhat.com>
>>
>> Clang static analysis reports this error
>> amdgpu_debugfs.c:1690:9: warning: 1st function call
>>    argument is an uninitialized value
>>    tmp = krealloc_array(tmp, i + 1,
>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> realloc will free tmp, so tmp can not be garbage.
>> And the return needs to be checked.
> Are you sure?
> A quick check seems to show that krealloc() behaves the same
> way as libc realloc() and the pointer isn't freed on failure.

I suck, I'll respin the patch

Thanks

Tom

>
> 	David
>
>> Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
>> Signed-off-by: Tom Rix <trix@redhat.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> index 9eb9b440bd438..159b97c0b4ebc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> @@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>>   {
>>   	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
>>   	char reg_offset[11];
>> -	uint32_t *tmp;
>> +	uint32_t *tmp = NULL;
>>   	int ret, i = 0, len = 0;
>>
>>   	do {
>> @@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>>   		}
>>
>>   		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
>> +		if (!tmp) {
>> +			ret = -ENOMEM;
>> +			goto error_free;
>> +		}
>>   		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
>>   			ret = -EINVAL;
>>   			goto error_free;
>> --
>> 2.26.3
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Fix realloc of ptr
@ 2022-02-27 14:36     ` Tom Rix
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Rix @ 2022-02-27 14:36 UTC (permalink / raw)
  To: David Laight, alexander.deucher, christian.koenig, Xinhui.Pan,
	airlied, daniel, nathan, ndesaulniers, nirmoy.das, lijo.lazar,
	tom.stdenis, evan.quan, kevin1.wang, Amaranath.Somalapuram
  Cc: llvm, dri-devel, amd-gfx, linux-kernel


On 2/26/22 2:21 PM, David Laight wrote:
> From: trix@redhat.com
>> Sent: 26 February 2022 15:59
>>
>> From: Tom Rix <trix@redhat.com>
>>
>> Clang static analysis reports this error
>> amdgpu_debugfs.c:1690:9: warning: 1st function call
>>    argument is an uninitialized value
>>    tmp = krealloc_array(tmp, i + 1,
>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> realloc will free tmp, so tmp can not be garbage.
>> And the return needs to be checked.
> Are you sure?
> A quick check seems to show that krealloc() behaves the same
> way as libc realloc() and the pointer isn't freed on failure.

I suck, I'll respin the patch

Thanks

Tom

>
> 	David
>
>> Fixes: 5ce5a584cb82 ("drm/amdgpu: add debugfs for reset registers list")
>> Signed-off-by: Tom Rix <trix@redhat.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> index 9eb9b440bd438..159b97c0b4ebc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>> @@ -1676,7 +1676,7 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>>   {
>>   	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
>>   	char reg_offset[11];
>> -	uint32_t *tmp;
>> +	uint32_t *tmp = NULL;
>>   	int ret, i = 0, len = 0;
>>
>>   	do {
>> @@ -1688,6 +1688,10 @@ static ssize_t amdgpu_reset_dump_register_list_write(struct file *f,
>>   		}
>>
>>   		tmp = krealloc_array(tmp, i + 1, sizeof(uint32_t), GFP_KERNEL);
>> +		if (!tmp) {
>> +			ret = -ENOMEM;
>> +			goto error_free;
>> +		}
>>   		if (sscanf(reg_offset, "%X %n", &tmp[i], &ret) != 1) {
>>   			ret = -EINVAL;
>>   			goto error_free;
>> --
>> 2.26.3
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-27 14:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-26 15:58 [PATCH] drm/amdgpu: Fix realloc of ptr trix
2022-02-26 15:58 ` trix
2022-02-26 22:21 ` David Laight
2022-02-26 22:21   ` David Laight
2022-02-27 14:36   ` Tom Rix
2022-02-27 14:36     ` Tom Rix

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.