* [PATCH] drm/amdgpu: fix memory leak
@ 2019-10-04 10:17 Nirmoy Das
2019-10-04 10:44 ` Koenig, Christian
0 siblings, 1 reply; 8+ messages in thread
From: Nirmoy Das @ 2019-10-04 10:17 UTC (permalink / raw)
To: alexander.deucher, christian.koenig
Cc: airlied, dri-devel, linux-kernel, nirmoy.das
In amdgpu_bo_list_ioctl when idr_alloc fails
don't return without freeing bo list entry.
Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 7bcf86c61999..c3e5ea544857 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
mutex_unlock(&fpriv->bo_list_lock);
if (r < 0) {
amdgpu_bo_list_put(list);
- return r;
+ goto error_free;
}
handle = r;
--
2.23.0
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 10:17 [PATCH] drm/amdgpu: fix memory leak Nirmoy Das
@ 2019-10-04 10:44 ` Koenig, Christian
2019-10-04 11:00 ` Nirmoy
0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2019-10-04 10:44 UTC (permalink / raw)
To: Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel, Das, Nirmoy
First of all please send mails regarding amdgpu to the amd-gfx mailing
list and not lkml/dri-devel.
Am 04.10.19 um 12:17 schrieb Nirmoy Das:
> In amdgpu_bo_list_ioctl when idr_alloc fails
> don't return without freeing bo list entry.
>
> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 7bcf86c61999..c3e5ea544857 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
> mutex_unlock(&fpriv->bo_list_lock);
> if (r < 0) {
> amdgpu_bo_list_put(list);
> - return r;
> + goto error_free;
NAK, that is a double free. The bo list entries are freed by
amdgpu_bo_list_put().
Regards,
Christian.
> }
>
> handle = r;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 10:44 ` Koenig, Christian
@ 2019-10-04 11:00 ` Nirmoy
2019-10-04 11:13 ` Koenig, Christian
0 siblings, 1 reply; 8+ messages in thread
From: Nirmoy @ 2019-10-04 11:00 UTC (permalink / raw)
To: Koenig, Christian, Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel, Das, Nirmoy
On 10/4/19 12:44 PM, Koenig, Christian wrote:
> First of all please send mails regarding amdgpu to the amd-gfx mailing
> list and not lkml/dri-devel.
Okay.
> Am 04.10.19 um 12:17 schrieb Nirmoy Das:
>> In amdgpu_bo_list_ioctl when idr_alloc fails
>> don't return without freeing bo list entry.
>>
>> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> index 7bcf86c61999..c3e5ea544857 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
>> mutex_unlock(&fpriv->bo_list_lock);
>> if (r < 0) {
>> amdgpu_bo_list_put(list);
>> - return r;
>> + goto error_free;
> NAK, that is a double free. The bo list entries are freed by
> amdgpu_bo_list_put().
Thanks, didn't realize that.
> Regards,
> Christian.
Regards,
Nirmoy
>> }
>>
>> handle = r;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 11:00 ` Nirmoy
@ 2019-10-04 11:13 ` Koenig, Christian
2019-10-04 11:26 ` Nirmoy
0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2019-10-04 11:13 UTC (permalink / raw)
To: Das, Nirmoy, Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel
Am 04.10.19 um 13:00 schrieb Das, Nirmoy:
> On 10/4/19 12:44 PM, Koenig, Christian wrote:
>> First of all please send mails regarding amdgpu to the amd-gfx mailing
>> list and not lkml/dri-devel.
> Okay.
>> Am 04.10.19 um 12:17 schrieb Nirmoy Das:
>>> In amdgpu_bo_list_ioctl when idr_alloc fails
>>> don't return without freeing bo list entry.
>>>
>>> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> index 7bcf86c61999..c3e5ea544857 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
>>> mutex_unlock(&fpriv->bo_list_lock);
>>> if (r < 0) {
>>> amdgpu_bo_list_put(list);
>>> - return r;
>>> + goto error_free;
>> NAK, that is a double free. The bo list entries are freed by
>> amdgpu_bo_list_put().
> Thanks, didn't realize that.
Wait a second, what entries are you talking about?
The entries in the list object are freed when amdgpu_bo_list_put() is
called, but the temporary info array with the handles needs to be freed
as well.
And it looks like that is indeed leaked here.
Regards,
Christian.
>> Regards,
>> Christian.
> Regards,
>
> Nirmoy
>
>>> }
>>>
>>> handle = r;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 11:13 ` Koenig, Christian
@ 2019-10-04 11:26 ` Nirmoy
2019-10-04 11:30 ` Koenig, Christian
0 siblings, 1 reply; 8+ messages in thread
From: Nirmoy @ 2019-10-04 11:26 UTC (permalink / raw)
To: Koenig, Christian, Das, Nirmoy, Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel
On 10/4/19 1:13 PM, Koenig, Christian wrote:
>
>>> NAK, that is a double free. The bo list entries are freed by
>>> amdgpu_bo_list_put().
>> Thanks, didn't realize that.
> Wait a second, what entries are you talking about?
>
> The entries in the list object are freed when amdgpu_bo_list_put() is
> called, but the temporary info array with the handles needs to be freed
> as well.
>
> And it looks like that is indeed leaked here.
I am talking about the `info` array created by
amdgpu_bo_create_list_entry_array().
> Regards,
> Christian.
>
>>> Regards,
>>> Christian.
>> Regards,
>>
>> Nirmoy
>>
>>>> }
>>>>
>>>> handle = r;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 11:26 ` Nirmoy
@ 2019-10-04 11:30 ` Koenig, Christian
2019-10-04 11:36 ` Nirmoy
0 siblings, 1 reply; 8+ messages in thread
From: Koenig, Christian @ 2019-10-04 11:30 UTC (permalink / raw)
To: Das, Nirmoy, Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel
Am 04.10.19 um 13:26 schrieb Das, Nirmoy:
> On 10/4/19 1:13 PM, Koenig, Christian wrote:
>>>> NAK, that is a double free. The bo list entries are freed by
>>>> amdgpu_bo_list_put().
>>> Thanks, didn't realize that.
>> Wait a second, what entries are you talking about?
>>
>> The entries in the list object are freed when amdgpu_bo_list_put() is
>> called, but the temporary info array with the handles needs to be freed
>> as well.
>>
>> And it looks like that is indeed leaked here.
> I am talking about the `info` array created by
> amdgpu_bo_create_list_entry_array().
Yeah, that are the handles and not the entries. Sorry that I was
confused about that.
Your patch is correct, you should just update the commit message a bit.
BTW: Could you cleanup error handling here a bit more?
E.g. add an error_put_list handle and drop the "if (info)" and instead
return directly if we fail to allocate info.
Thanks,
Christian.
>> Regards,
>> Christian.
>>
>>>> Regards,
>>>> Christian.
>>> Regards,
>>>
>>> Nirmoy
>>>
>>>>> }
>>>>>
>>>>> handle = r;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/amdgpu: fix memory leak
2019-10-04 11:30 ` Koenig, Christian
@ 2019-10-04 11:36 ` Nirmoy
0 siblings, 0 replies; 8+ messages in thread
From: Nirmoy @ 2019-10-04 11:36 UTC (permalink / raw)
To: Koenig, Christian, Das, Nirmoy, Nirmoy Das, Deucher, Alexander
Cc: airlied, dri-devel, linux-kernel
On 10/4/19 1:30 PM, Koenig, Christian wrote:
> Am 04.10.19 um 13:26 schrieb Das, Nirmoy:
>> On 10/4/19 1:13 PM, Koenig, Christian wrote:
>>>>> NAK, that is a double free. The bo list entries are freed by
>>>>> amdgpu_bo_list_put().
>>>> Thanks, didn't realize that.
>>> Wait a second, what entries are you talking about?
>>>
>>> The entries in the list object are freed when amdgpu_bo_list_put() is
>>> called, but the temporary info array with the handles needs to be freed
>>> as well.
>>>
>>> And it looks like that is indeed leaked here.
>> I am talking about the `info` array created by
>> amdgpu_bo_create_list_entry_array().
> Yeah, that are the handles and not the entries. Sorry that I was
> confused about that.
>
> Your patch is correct, you should just update the commit message a bit.
>
> BTW: Could you cleanup error handling here a bit more?
>
> E.g. add an error_put_list handle and drop the "if (info)" and instead
> return directly if we fail to allocate info.
Okay I will do that in v2 of this patch.
> Thanks,
> Christian.
Regards,
Nirmoy
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/amdgpu: fix memory leak
@ 2015-09-18 8:46 Sudip Mukherjee
0 siblings, 0 replies; 8+ messages in thread
From: Sudip Mukherjee @ 2015-09-18 8:46 UTC (permalink / raw)
To: David Airlie, Christian König
Cc: linux-kernel, dri-devel, Sudip Mukherjee
If amdgpu_ib_get() fails we returned the error code but we missed
freeing ib.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f68b7cd..0e61074 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -455,8 +455,10 @@ int amdgpu_vm_update_page_directory(struct amdgpu_device *adev,
return -ENOMEM;
r = amdgpu_ib_get(ring, NULL, ndw * 4, ib);
- if (r)
+ if (r) {
+ kfree(ib);
return r;
+ }
ib->length_dw = 0;
/* walk over the address space and update the page directory */
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-04 11:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 10:17 [PATCH] drm/amdgpu: fix memory leak Nirmoy Das
2019-10-04 10:44 ` Koenig, Christian
2019-10-04 11:00 ` Nirmoy
2019-10-04 11:13 ` Koenig, Christian
2019-10-04 11:26 ` Nirmoy
2019-10-04 11:30 ` Koenig, Christian
2019-10-04 11:36 ` Nirmoy
-- strict thread matches above, loose matches on Subject: below --
2015-09-18 8:46 Sudip Mukherjee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®