[PATCH] drm/syncobj: fix leaking dma_fence in drm_syncobj_query_ioctl

[PATCH] drm/syncobj: fix leaking dma_fence in drm_syncobj_query_ioctl

[Christian König]

We need to check the context number instead if the previous sequence to detect
an error and if an error is detected we need to drop the reference to the
current fence or otherwise would leak it.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_syncobj.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 75cb4bb7619e..1438dcb3ebb1 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1298,14 +1298,14 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 			struct dma_fence *iter, *last_signaled = NULL;
 
 			dma_fence_chain_for_each(iter, fence) {
-				if (!iter)
-					break;
-				dma_fence_put(last_signaled);
-				last_signaled = dma_fence_get(iter);
-				if (!to_dma_fence_chain(last_signaled)->prev_seqno)
+				if (iter->context != fence->context) {
+					dma_fence_put(iter);
 					/* It is most likely that timeline has
 					 * unorder points. */
 					break;
+				}
+				dma_fence_put(last_signaled);
+				last_signaled = dma_fence_get(iter);
 			}
 			point = dma_fence_is_signaled(last_signaled) ?
 				last_signaled->seqno :
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/syncobj: fix leaking dma_fence in drm_syncobj_query_ioctl

[Lionel Landwerlin]

On 22/07/2019 15:59, Christian König wrote:
> We need to check the context number instead if the previous sequence to detect
> an error and if an error is detected we need to drop the reference to the
> current fence or otherwise would leak it.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6")
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

> ---
>   drivers/gpu/drm/drm_syncobj.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 75cb4bb7619e..1438dcb3ebb1 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1298,14 +1298,14 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>   			struct dma_fence *iter, *last_signaled = NULL;
>   
>   			dma_fence_chain_for_each(iter, fence) {
> -				if (!iter)
> -					break;
> -				dma_fence_put(last_signaled);
> -				last_signaled = dma_fence_get(iter);
> -				if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> +				if (iter->context != fence->context) {
> +					dma_fence_put(iter);
>   					/* It is most likely that timeline has
>   					 * unorder points. */
>   					break;
> +				}
> +				dma_fence_put(last_signaled);
> +				last_signaled = dma_fence_get(iter);
>   			}
>   			point = dma_fence_is_signaled(last_signaled) ?
>   				last_signaled->seqno :


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: fix leaking dma_fence in drm_syncobj_query_ioctl

[Christian König]

Am 22.07.19 um 15:16 schrieb Lionel Landwerlin:
> On 22/07/2019 15:59, Christian König wrote:
>> We need to check the context number instead if the previous sequence 
>> to detect
>> an error and if an error is detected we need to drop the reference to 
>> the
>> current fence or otherwise would leak it.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>
> Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6")
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

CC stable? I'm not sure when this got upstream.

Christian.

>
>> ---
>>   drivers/gpu/drm/drm_syncobj.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index 75cb4bb7619e..1438dcb3ebb1 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1298,14 +1298,14 @@ int drm_syncobj_query_ioctl(struct drm_device 
>> *dev, void *data,
>>               struct dma_fence *iter, *last_signaled = NULL;
>>                 dma_fence_chain_for_each(iter, fence) {
>> -                if (!iter)
>> -                    break;
>> -                dma_fence_put(last_signaled);
>> -                last_signaled = dma_fence_get(iter);
>> -                if (!to_dma_fence_chain(last_signaled)->prev_seqno)
>> +                if (iter->context != fence->context) {
>> +                    dma_fence_put(iter);
>>                       /* It is most likely that timeline has
>>                        * unorder points. */
>>                       break;
>> +                }
>> +                dma_fence_put(last_signaled);
>> +                last_signaled = dma_fence_get(iter);
>>               }
>>               point = dma_fence_is_signaled(last_signaled) ?
>>                   last_signaled->seqno :
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: fix leaking dma_fence in drm_syncobj_query_ioctl

[Lionel Landwerlin]

On 22/07/2019 16:21, Christian König wrote:
> Am 22.07.19 um 15:16 schrieb Lionel Landwerlin:
>> On 22/07/2019 15:59, Christian König wrote:
>>> We need to check the context number instead if the previous sequence 
>>> to detect
>>> an error and if an error is detected we need to drop the reference 
>>> to the
>>> current fence or otherwise would leak it.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>
>> Fixes: 27b575a9aa2f ("drm/syncobj: add timeline payload query ioctl v6")
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>
> CC stable? I'm not sure when this got upstream.
>
> Christian.


I thought it would get picked up automatically for the relevant stable 
version (and none if it's not upstream yet).

We also have to be nice to people cherry picking stuff like ChromeOS.


-Lionel


>
>>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 10 +++++-----
>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 75cb4bb7619e..1438dcb3ebb1 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -1298,14 +1298,14 @@ int drm_syncobj_query_ioctl(struct 
>>> drm_device *dev, void *data,
>>>               struct dma_fence *iter, *last_signaled = NULL;
>>>                 dma_fence_chain_for_each(iter, fence) {
>>> -                if (!iter)
>>> -                    break;
>>> -                dma_fence_put(last_signaled);
>>> -                last_signaled = dma_fence_get(iter);
>>> -                if (!to_dma_fence_chain(last_signaled)->prev_seqno)
>>> +                if (iter->context != fence->context) {
>>> +                    dma_fence_put(iter);
>>>                       /* It is most likely that timeline has
>>>                        * unorder points. */
>>>                       break;
>>> +                }
>>> +                dma_fence_put(last_signaled);
>>> +                last_signaled = dma_fence_get(iter);
>>>               }
>>>               point = dma_fence_is_signaled(last_signaled) ?
>>>                   last_signaled->seqno :
>>
>>
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx