* [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init
@ 2024-03-21 14:56 Maarten Lankhorst
2024-03-21 19:56 ` Rodrigo Vivi
0 siblings, 1 reply; 5+ messages in thread
From: Maarten Lankhorst @ 2024-03-21 14:56 UTC (permalink / raw)
To: intel-xe; +Cc: Maarten Lankhorst, stable
Add a reference to bo after all error paths, to prevent leaking a bo
ref.
Return 0 to clarify that this is the success path.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
Cc: <stable@vger.kernel.org> # v6.8+
---
drivers/gpu/drm/xe/display/intel_fb_bo.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
index b21da7b745a5..7262bbca9baf 100644
--- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
@@ -27,8 +27,6 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
struct drm_i915_private *i915 = to_i915(bo->ttm.base.dev);
int ret;
- xe_bo_get(bo);
-
ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
if (ret)
return ret;
@@ -48,7 +46,8 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
}
ttm_bo_unreserve(&bo->ttm);
- return ret;
+ xe_bo_get(bo);
+ return 0;
}
struct xe_bo *intel_fb_bo_lookup_valid_bo(struct drm_i915_private *i915,
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init
2024-03-21 14:56 [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init Maarten Lankhorst
@ 2024-03-21 19:56 ` Rodrigo Vivi
2024-03-22 16:16 ` Lucas De Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Rodrigo Vivi @ 2024-03-21 19:56 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-xe, stable
On Thu, Mar 21, 2024 at 03:56:44PM +0100, Maarten Lankhorst wrote:
> Add a reference to bo after all error paths, to prevent leaking a bo
> ref.
>
> Return 0 to clarify that this is the success path.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
> Cc: <stable@vger.kernel.org> # v6.8+
> ---
> drivers/gpu/drm/xe/display/intel_fb_bo.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
> index b21da7b745a5..7262bbca9baf 100644
> --- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
> +++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
> @@ -27,8 +27,6 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
> struct drm_i915_private *i915 = to_i915(bo->ttm.base.dev);
> int ret;
>
> - xe_bo_get(bo);
> -
> ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
> if (ret)
> return ret;
> @@ -48,7 +46,8 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
> }
> ttm_bo_unreserve(&bo->ttm);
>
> - return ret;
> + xe_bo_get(bo);
wouldn't be safer to keep the get in the beginning of everything else
and then if in an error path you xe_bo_put(bo); ?!
> + return 0;
> }
>
> struct xe_bo *intel_fb_bo_lookup_valid_bo(struct drm_i915_private *i915,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init
2024-03-21 19:56 ` Rodrigo Vivi
@ 2024-03-22 16:16 ` Lucas De Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Lucas De Marchi @ 2024-03-22 16:16 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Maarten Lankhorst, intel-xe, stable
On Thu, Mar 21, 2024 at 03:56:23PM -0400, Rodrigo Vivi wrote:
>On Thu, Mar 21, 2024 at 03:56:44PM +0100, Maarten Lankhorst wrote:
>> Add a reference to bo after all error paths, to prevent leaking a bo
>> ref.
>>
>> Return 0 to clarify that this is the success path.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
>> Cc: <stable@vger.kernel.org> # v6.8+
>> ---
>> drivers/gpu/drm/xe/display/intel_fb_bo.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> index b21da7b745a5..7262bbca9baf 100644
>> --- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> +++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
>> @@ -27,8 +27,6 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
>> struct drm_i915_private *i915 = to_i915(bo->ttm.base.dev);
>> int ret;
>>
>> - xe_bo_get(bo);
>> -
>> ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
>> if (ret)
>> return ret;
>> @@ -48,7 +46,8 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
>> }
>> ttm_bo_unreserve(&bo->ttm);
>>
>> - return ret;
>> + xe_bo_get(bo);
>
>wouldn't be safer to keep the get in the beginning of everything else
>and then if in an error path you xe_bo_put(bo); ?!
yes, I was thinking exactly that. Otherwise it's harder to reason about
the lifetime of the object and why the bo couldn't disappear after e.g.
ttm_bo_reserve() and cause use-after-free.
Lucas De Marchi
>
>> + return 0;
>> }
>>
>> struct xe_bo *intel_fb_bo_lookup_valid_bo(struct drm_i915_private *i915,
>> --
>> 2.43.0
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init
@ 2024-04-04 9:03 Maarten Lankhorst
2024-04-08 10:13 ` Nirmoy Das
0 siblings, 1 reply; 5+ messages in thread
From: Maarten Lankhorst @ 2024-04-04 9:03 UTC (permalink / raw)
To: intel-xe; +Cc: Maarten Lankhorst, stable
Add a unreference bo in the error path, to prevent leaking a bo ref.
Return 0 on success to clarify the success path.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
Cc: <stable@vger.kernel.org> # v6.8+
---
drivers/gpu/drm/xe/display/intel_fb_bo.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
index dba327f53ac5..e18521acc516 100644
--- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
@@ -31,7 +31,7 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
if (ret)
- return ret;
+ goto err;
if (!(bo->flags & XE_BO_FLAG_SCANOUT)) {
/*
@@ -42,12 +42,16 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
*/
if (XE_IOCTL_DBG(i915, !list_empty(&bo->ttm.base.gpuva.list))) {
ttm_bo_unreserve(&bo->ttm);
- return -EINVAL;
+ ret = -EINVAL;
+ goto err;
}
bo->flags |= XE_BO_FLAG_SCANOUT;
}
ttm_bo_unreserve(&bo->ttm);
+ return 0;
+err:
+ xe_bo_put(bo);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init
2024-04-04 9:03 Maarten Lankhorst
@ 2024-04-08 10:13 ` Nirmoy Das
0 siblings, 0 replies; 5+ messages in thread
From: Nirmoy Das @ 2024-04-08 10:13 UTC (permalink / raw)
To: Maarten Lankhorst, intel-xe; +Cc: stable
On 4/4/2024 11:03 AM, Maarten Lankhorst wrote:
> Add a unreference bo in the error path, to prevent leaking a bo ref.
>
> Return 0 on success to clarify the success path.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
> Cc: <stable@vger.kernel.org> # v6.8+
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
> drivers/gpu/drm/xe/display/intel_fb_bo.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
> index dba327f53ac5..e18521acc516 100644
> --- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
> +++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
> @@ -31,7 +31,7 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
>
> ret = ttm_bo_reserve(&bo->ttm, true, false, NULL);
> if (ret)
> - return ret;
> + goto err;
>
> if (!(bo->flags & XE_BO_FLAG_SCANOUT)) {
> /*
> @@ -42,12 +42,16 @@ int intel_fb_bo_framebuffer_init(struct intel_framebuffer *intel_fb,
> */
> if (XE_IOCTL_DBG(i915, !list_empty(&bo->ttm.base.gpuva.list))) {
> ttm_bo_unreserve(&bo->ttm);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto err;
> }
> bo->flags |= XE_BO_FLAG_SCANOUT;
> }
> ttm_bo_unreserve(&bo->ttm);
> + return 0;
>
> +err:
> + xe_bo_put(bo);
> return ret;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-08 10:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-21 14:56 [PATCH] drm/xe: Fix bo leak in intel_fb_bo_framebuffer_init Maarten Lankhorst
2024-03-21 19:56 ` Rodrigo Vivi
2024-03-22 16:16 ` Lucas De Marchi
-- strict thread matches above, loose matches on Subject: below --
2024-04-04 9:03 Maarten Lankhorst
2024-04-08 10:13 ` Nirmoy Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).