[PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[Matthew Auld]

There look to be some nasty races here when triggering the
invalidate_mappings hook:

1) We do xe_bo_alloc() followed by the attach, before the actual full bo
   init step in xe_dma_buf_init_obj(). However the bo is visible on the
   attachments list after the attach.  This is bad since exporter driver,
   say amdgpu, can at any time call back into our invalidate_mappings hook,
   with an empty/bogus bo, leading to potential bugs/crashes.

2) Similar to 1) but here we get a UAF, when the invalidate_mappings
   hook is triggered. For example, we get as far as xe_bo_init_locked()
   but this fails in some way. But here the bo will be freed on error, but
   we still have it attached from dma-buf pov, so if the
   invalidate_mappings is now triggered then the bo we access is gone and
   we trigger UAF and more bugs/crashes.

To fix this, move the attach step until after we actually have a fully
set up buffer object. Note that the bo is not published to userspace
until later, so not sure what the comment "Don't publish the bo
until we have a valid attachment", is referring to.

We have at least two different customers reporting hitting a NULL ptr
deref in evict_flags when importing something from amdgpu, followed by
triggering the evict flow. Hit rate is also pretty low, which would
hint at some kind of race, so something like 1) or 2) might explain
this.

Assisted-by: Gemini:gemini-3 #debug
Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7903
Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4055
Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: <stable@vger.kernel.org> # v6.8+
---
 drivers/gpu/drm/xe/xe_dma_buf.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
index b9828da15897..e6c2f7d30abb 100644
--- a/drivers/gpu/drm/xe/xe_dma_buf.c
+++ b/drivers/gpu/drm/xe/xe_dma_buf.c
@@ -357,11 +357,6 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
 		}
 	}
 
-	/*
-	 * Don't publish the bo until we have a valid attachment, and a
-	 * valid attachment needs the bo address. So pre-create a bo before
-	 * creating the attachment and publish.
-	 */
 	bo = xe_bo_alloc();
 	if (IS_ERR(bo))
 		return ERR_CAST(bo);
@@ -371,6 +366,13 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
 	if (test)
 		attach_ops = test->attach_ops;
 #endif
+	/*
+	 * xe_dma_buf_init_obj() takes ownership of bo on both success
+	 * and failure, so we must not touch bo after this call.
+	 */
+	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
+	if (IS_ERR(obj))
+		return obj;
 
 	attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops, &bo->ttm.base);
 	if (IS_ERR(attach)) {
@@ -378,21 +380,12 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
 		goto out_err;
 	}
 
-	/*
-	 * xe_dma_buf_init_obj() takes ownership of bo on both success
-	 * and failure, so we must not touch bo after this call.
-	 */
-	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
-	if (IS_ERR(obj)) {
-		dma_buf_detach(dma_buf, attach);
-		return obj;
-	}
 	get_dma_buf(dma_buf);
 	obj->import_attach = attach;
 	return obj;
 
 out_err:
-	xe_bo_free(bo);
+	xe_bo_put(bo);
 
 	return obj;
 }
-- 
2.53.0

Re: [PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[Matthew Brost]

On Wed, May 06, 2026 at 07:43:33PM +0100, Matthew Auld wrote:
> There look to be some nasty races here when triggering the
> invalidate_mappings hook:
> 
> 1) We do xe_bo_alloc() followed by the attach, before the actual full bo
>    init step in xe_dma_buf_init_obj(). However the bo is visible on the
>    attachments list after the attach.  This is bad since exporter driver,
>    say amdgpu, can at any time call back into our invalidate_mappings hook,
>    with an empty/bogus bo, leading to potential bugs/crashes.
> 
> 2) Similar to 1) but here we get a UAF, when the invalidate_mappings
>    hook is triggered. For example, we get as far as xe_bo_init_locked()
>    but this fails in some way. But here the bo will be freed on error, but
>    we still have it attached from dma-buf pov, so if the
>    invalidate_mappings is now triggered then the bo we access is gone and
>    we trigger UAF and more bugs/crashes.
> 
> To fix this, move the attach step until after we actually have a fully
> set up buffer object. Note that the bo is not published to userspace
> until later, so not sure what the comment "Don't publish the bo
> until we have a valid attachment", is referring to.
> 
> We have at least two different customers reporting hitting a NULL ptr
> deref in evict_flags when importing something from amdgpu, followed by
> triggering the evict flow. Hit rate is also pretty low, which would
> hint at some kind of race, so something like 1) or 2) might explain
> this.
> 
> Assisted-by: Gemini:gemini-3 #debug
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7903
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4055
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

One suggestion...

> Cc: <stable@vger.kernel.org> # v6.8+
> ---
>  drivers/gpu/drm/xe/xe_dma_buf.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
> index b9828da15897..e6c2f7d30abb 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -357,11 +357,6 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
>  		}
>  	}
>  
> -	/*
> -	 * Don't publish the bo until we have a valid attachment, and a
> -	 * valid attachment needs the bo address. So pre-create a bo before
> -	 * creating the attachment and publish.
> -	 */
>  	bo = xe_bo_alloc();
>  	if (IS_ERR(bo))
>  		return ERR_CAST(bo);
> @@ -371,6 +366,13 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
>  	if (test)
>  		attach_ops = test->attach_ops;
>  #endif
> +	/*
> +	 * xe_dma_buf_init_obj() takes ownership of bo on both success
> +	 * and failure, so we must not touch bo after this call.

Maybe quick comment indicating something like in the commit message why
this must be done before dma_buf_dynamic_attach.

Matt

> +	 */
> +	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> +	if (IS_ERR(obj))
> +		return obj;
>  
>  	attach = dma_buf_dynamic_attach(dma_buf, dev->dev, attach_ops, &bo->ttm.base);
>  	if (IS_ERR(attach)) {
> @@ -378,21 +380,12 @@ struct drm_gem_object *xe_gem_prime_import(struct drm_device *dev,
>  		goto out_err;
>  	}
>  
> -	/*
> -	 * xe_dma_buf_init_obj() takes ownership of bo on both success
> -	 * and failure, so we must not touch bo after this call.
> -	 */
> -	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> -	if (IS_ERR(obj)) {
> -		dma_buf_detach(dma_buf, attach);
> -		return obj;
> -	}
>  	get_dma_buf(dma_buf);
>  	obj->import_attach = attach;
>  	return obj;
>  
>  out_err:
> -	xe_bo_free(bo);
> +	xe_bo_put(bo);
>  
>  	return obj;
>  }
> -- 
> 2.53.0
> 

Re: [PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[Thomas Hellström]

On Wed, 2026-05-06 at 19:43 +0100, Matthew Auld wrote:
> There look to be some nasty races here when triggering the
> invalidate_mappings hook:
> 
> 1) We do xe_bo_alloc() followed by the attach, before the actual full
> bo
>    init step in xe_dma_buf_init_obj(). However the bo is visible on
> the
>    attachments list after the attach.  This is bad since exporter
> driver,
>    say amdgpu, can at any time call back into our invalidate_mappings
> hook,
>    with an empty/bogus bo, leading to potential bugs/crashes.
> 
> 2) Similar to 1) but here we get a UAF, when the invalidate_mappings
>    hook is triggered. For example, we get as far as
> xe_bo_init_locked()
>    but this fails in some way. But here the bo will be freed on
> error, but
>    we still have it attached from dma-buf pov, so if the
>    invalidate_mappings is now triggered then the bo we access is gone
> and
>    we trigger UAF and more bugs/crashes.
> 
> To fix this, move the attach step until after we actually have a
> fully
> set up buffer object. Note that the bo is not published to userspace
> until later, so not sure what the comment "Don't publish the bo
> until we have a valid attachment", is referring to.
> 
> We have at least two different customers reporting hitting a NULL ptr
> deref in evict_flags when importing something from amdgpu, followed
> by
> triggering the evict flow. Hit rate is also pretty low, which would
> hint at some kind of race, so something like 1) or 2) might explain
> this.
> 
> Assisted-by: Gemini:gemini-3 #debug
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7903
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4055
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> GPUs")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: <stable@vger.kernel.org> # v6.8+
> ---
>  drivers/gpu/drm/xe/xe_dma_buf.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> b/drivers/gpu/drm/xe/xe_dma_buf.c
> index b9828da15897..e6c2f7d30abb 100644
> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> @@ -357,11 +357,6 @@ struct drm_gem_object
> *xe_gem_prime_import(struct drm_device *dev,
>  		}
>  	}
>  
> -	/*
> -	 * Don't publish the bo until we have a valid attachment,
> and a
> -	 * valid attachment needs the bo address. So pre-create a bo
> before
> -	 * creating the attachment and publish.
> -	 */
>  	bo = xe_bo_alloc();
>  	if (IS_ERR(bo))
>  		return ERR_CAST(bo);
> @@ -371,6 +366,13 @@ struct drm_gem_object
> *xe_gem_prime_import(struct drm_device *dev,
>  	if (test)
>  		attach_ops = test->attach_ops;
>  #endif
> +	/*
> +	 * xe_dma_buf_init_obj() takes ownership of bo on both
> success
> +	 * and failure, so we must not touch bo after this call.
> +	 */
> +	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> +	if (IS_ERR(obj))
> +		return obj;

IIRC this publishes the bo on the LRUs, as per the removed comment.
What happens if, for example, the shrinker kicks in and shrinks it? But
similarly perhaps we should have obj->import_attach set already at
publish time?

If this is indeed the case we might have to revert to some trickery.
Like invalidate_mappings() returning early if the init is not complete,
and set obj->import_attach under the lock in xe_dma_buf_init_obj?

Also I think IIRC xe_bo_alloc() was created specifically for this
situation, so unless there are more users of that, and the ordering in
this patch is indeed correct, we might be able to get rid of the two-
step bo creation here.

/Thomas


>  
>  	attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
> attach_ops, &bo->ttm.base);
>  	if (IS_ERR(attach)) {
> @@ -378,21 +380,12 @@ struct drm_gem_object
> *xe_gem_prime_import(struct drm_device *dev,
>  		goto out_err;
>  	}
>  
> -	/*
> -	 * xe_dma_buf_init_obj() takes ownership of bo on both
> success
> -	 * and failure, so we must not touch bo after this call.
> -	 */
> -	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> -	if (IS_ERR(obj)) {
> -		dma_buf_detach(dma_buf, attach);
> -		return obj;
> -	}
>  	get_dma_buf(dma_buf);
>  	obj->import_attach = attach;
>  	return obj;
>  
>  out_err:
> -	xe_bo_free(bo);
> +	xe_bo_put(bo);
>  
>  	return obj;
>  }

Re: [PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[Matthew Auld]

On 06/05/2026 20:59, Thomas Hellström wrote:
> On Wed, 2026-05-06 at 19:43 +0100, Matthew Auld wrote:
>> There look to be some nasty races here when triggering the
>> invalidate_mappings hook:
>>
>> 1) We do xe_bo_alloc() followed by the attach, before the actual full
>> bo
>>     init step in xe_dma_buf_init_obj(). However the bo is visible on
>> the
>>     attachments list after the attach.  This is bad since exporter
>> driver,
>>     say amdgpu, can at any time call back into our invalidate_mappings
>> hook,
>>     with an empty/bogus bo, leading to potential bugs/crashes.
>>
>> 2) Similar to 1) but here we get a UAF, when the invalidate_mappings
>>     hook is triggered. For example, we get as far as
>> xe_bo_init_locked()
>>     but this fails in some way. But here the bo will be freed on
>> error, but
>>     we still have it attached from dma-buf pov, so if the
>>     invalidate_mappings is now triggered then the bo we access is gone
>> and
>>     we trigger UAF and more bugs/crashes.
>>
>> To fix this, move the attach step until after we actually have a
>> fully
>> set up buffer object. Note that the bo is not published to userspace
>> until later, so not sure what the comment "Don't publish the bo
>> until we have a valid attachment", is referring to.
>>
>> We have at least two different customers reporting hitting a NULL ptr
>> deref in evict_flags when importing something from amdgpu, followed
>> by
>> triggering the evict flow. Hit rate is also pretty low, which would
>> hint at some kind of race, so something like 1) or 2) might explain
>> this.
>>
>> Assisted-by: Gemini:gemini-3 #debug
>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7903
>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4055
>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
>> GPUs")
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.8+
>> ---
>>   drivers/gpu/drm/xe/xe_dma_buf.c | 23 ++++++++---------------
>>   1 file changed, 8 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
>> b/drivers/gpu/drm/xe/xe_dma_buf.c
>> index b9828da15897..e6c2f7d30abb 100644
>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c
>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
>> @@ -357,11 +357,6 @@ struct drm_gem_object
>> *xe_gem_prime_import(struct drm_device *dev,
>>   		}
>>   	}
>>   
>> -	/*
>> -	 * Don't publish the bo until we have a valid attachment,
>> and a
>> -	 * valid attachment needs the bo address. So pre-create a bo
>> before
>> -	 * creating the attachment and publish.
>> -	 */
>>   	bo = xe_bo_alloc();
>>   	if (IS_ERR(bo))
>>   		return ERR_CAST(bo);
>> @@ -371,6 +366,13 @@ struct drm_gem_object
>> *xe_gem_prime_import(struct drm_device *dev,
>>   	if (test)
>>   		attach_ops = test->attach_ops;
>>   #endif
>> +	/*
>> +	 * xe_dma_buf_init_obj() takes ownership of bo on both
>> success
>> +	 * and failure, so we must not touch bo after this call.
>> +	 */
>> +	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
>> +	if (IS_ERR(obj))
>> +		return obj;
> 
> IIRC this publishes the bo on the LRUs, as per the removed comment.
> What happens if, for example, the shrinker kicks in and shrinks it? But
> similarly perhaps we should have obj->import_attach set already at
> publish time?

I don't think anything bad will happen? I would view it as an sg object 
without any real backing store or attachment. Trying to 
move/shrink/evict should be a noop, like moving from SYS -> SYS 
(starting placement for type_sg). But since this a type_sg bo, I think 
shrinker will already ignore it right?

 From user POV, the handle is only published until much later at the end 
of drm_gem_prime_fd_to_handle(), after our import callback here, AFAICT. 
So I don't think there is a risk of the user somehow using the imported 
bo in an ioctl, before we have done the attach etc.

One other data point is perhaps amdgpu, which does seem to do the create 
+ attach as normal steps.

> 
> If this is indeed the case we might have to revert to some trickery.
> Like invalidate_mappings() returning early if the init is not complete,
> and set obj->import_attach under the lock in xe_dma_buf_init_obj?
> 
> Also I think IIRC xe_bo_alloc() was created specifically for this
> situation, so unless there are more users of that, and the ordering in
> this patch is indeed correct, we might be able to get rid of the two-
> step bo creation here.
> 
> /Thomas
> 
> 
>>   
>>   	attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
>> attach_ops, &bo->ttm.base);
>>   	if (IS_ERR(attach)) {
>> @@ -378,21 +380,12 @@ struct drm_gem_object
>> *xe_gem_prime_import(struct drm_device *dev,
>>   		goto out_err;
>>   	}
>>   
>> -	/*
>> -	 * xe_dma_buf_init_obj() takes ownership of bo on both
>> success
>> -	 * and failure, so we must not touch bo after this call.
>> -	 */
>> -	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
>> -	if (IS_ERR(obj)) {
>> -		dma_buf_detach(dma_buf, attach);
>> -		return obj;
>> -	}
>>   	get_dma_buf(dma_buf);
>>   	obj->import_attach = attach;
>>   	return obj;
>>   
>>   out_err:
>> -	xe_bo_free(bo);
>> +	xe_bo_put(bo);
>>   
>>   	return obj;
>>   }

Re: [PATCH] drm/xe/dma-buf: handle empty bo and UAF races

[Thomas Hellström]

On Thu, 2026-05-07 at 10:03 +0100, Matthew Auld wrote:
> On 06/05/2026 20:59, Thomas Hellström wrote:
> > On Wed, 2026-05-06 at 19:43 +0100, Matthew Auld wrote:
> > > There look to be some nasty races here when triggering the
> > > invalidate_mappings hook:
> > > 
> > > 1) We do xe_bo_alloc() followed by the attach, before the actual
> > > full
> > > bo
> > >     init step in xe_dma_buf_init_obj(). However the bo is visible
> > > on
> > > the
> > >     attachments list after the attach.  This is bad since
> > > exporter
> > > driver,
> > >     say amdgpu, can at any time call back into our
> > > invalidate_mappings
> > > hook,
> > >     with an empty/bogus bo, leading to potential bugs/crashes.
> > > 
> > > 2) Similar to 1) but here we get a UAF, when the
> > > invalidate_mappings
> > >     hook is triggered. For example, we get as far as
> > > xe_bo_init_locked()
> > >     but this fails in some way. But here the bo will be freed on
> > > error, but
> > >     we still have it attached from dma-buf pov, so if the
> > >     invalidate_mappings is now triggered then the bo we access is
> > > gone
> > > and
> > >     we trigger UAF and more bugs/crashes.
> > > 
> > > To fix this, move the attach step until after we actually have a
> > > fully
> > > set up buffer object. Note that the bo is not published to
> > > userspace
> > > until later, so not sure what the comment "Don't publish the bo
> > > until we have a valid attachment", is referring to.
> > > 
> > > We have at least two different customers reporting hitting a NULL
> > > ptr
> > > deref in evict_flags when importing something from amdgpu,
> > > followed
> > > by
> > > triggering the evict flow. Hit rate is also pretty low, which
> > > would
> > > hint at some kind of race, so something like 1) or 2) might
> > > explain
> > > this.
> > > 
> > > Assisted-by: Gemini:gemini-3 #debug
> > > Link:
> > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/7903
> > > Link:
> > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/4055
> > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for
> > > Intel
> > > GPUs")
> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > Cc: <stable@vger.kernel.org> # v6.8+
> > > ---
> > >   drivers/gpu/drm/xe/xe_dma_buf.c | 23 ++++++++---------------
> > >   1 file changed, 8 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > b/drivers/gpu/drm/xe/xe_dma_buf.c
> > > index b9828da15897..e6c2f7d30abb 100644
> > > --- a/drivers/gpu/drm/xe/xe_dma_buf.c
> > > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c
> > > @@ -357,11 +357,6 @@ struct drm_gem_object
> > > *xe_gem_prime_import(struct drm_device *dev,
> > >   		}
> > >   	}
> > >   
> > > -	/*
> > > -	 * Don't publish the bo until we have a valid
> > > attachment,
> > > and a
> > > -	 * valid attachment needs the bo address. So pre-create
> > > a bo
> > > before
> > > -	 * creating the attachment and publish.
> > > -	 */
> > >   	bo = xe_bo_alloc();
> > >   	if (IS_ERR(bo))
> > >   		return ERR_CAST(bo);
> > > @@ -371,6 +366,13 @@ struct drm_gem_object
> > > *xe_gem_prime_import(struct drm_device *dev,
> > >   	if (test)
> > >   		attach_ops = test->attach_ops;
> > >   #endif
> > > +	/*
> > > +	 * xe_dma_buf_init_obj() takes ownership of bo on both
> > > success
> > > +	 * and failure, so we must not touch bo after this call.
> > > +	 */
> > > +	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> > > +	if (IS_ERR(obj))
> > > +		return obj;
> > 
> > IIRC this publishes the bo on the LRUs, as per the removed comment.
> > What happens if, for example, the shrinker kicks in and shrinks it?
> > But
> > similarly perhaps we should have obj->import_attach set already at
> > publish time?
> 
> I don't think anything bad will happen? I would view it as an sg
> object 
> without any real backing store or attachment. Trying to 
> move/shrink/evict should be a noop, like moving from SYS -> SYS 
> (starting placement for type_sg). But since this a type_sg bo, I
> think 
> shrinker will already ignore it right?
> 
>  From user POV, the handle is only published until much later at the
> end 
> of drm_gem_prime_fd_to_handle(), after our import callback here,
> AFAICT. 
> So I don't think there is a risk of the user somehow using the
> imported 
> bo in an ioctl, before we have done the attach etc.
> 
> One other data point is perhaps amdgpu, which does seem to do the
> create 
> + attach as normal steps.

OK, Yeah if there's no way the bo can see other unexpected callbacks
after initializing it, then

Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> 
> > 
> > If this is indeed the case we might have to revert to some
> > trickery.
> > Like invalidate_mappings() returning early if the init is not
> > complete,
> > and set obj->import_attach under the lock in xe_dma_buf_init_obj?
> > 
> > Also I think IIRC xe_bo_alloc() was created specifically for this
> > situation, so unless there are more users of that, and the ordering
> > in
> > this patch is indeed correct, we might be able to get rid of the
> > two-
> > step bo creation here.
> > 
> > /Thomas
> > 
> > 
> > >   
> > >   	attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
> > > attach_ops, &bo->ttm.base);
> > >   	if (IS_ERR(attach)) {
> > > @@ -378,21 +380,12 @@ struct drm_gem_object
> > > *xe_gem_prime_import(struct drm_device *dev,
> > >   		goto out_err;
> > >   	}
> > >   
> > > -	/*
> > > -	 * xe_dma_buf_init_obj() takes ownership of bo on both
> > > success
> > > -	 * and failure, so we must not touch bo after this call.
> > > -	 */
> > > -	obj = xe_dma_buf_init_obj(dev, bo, dma_buf);
> > > -	if (IS_ERR(obj)) {
> > > -		dma_buf_detach(dma_buf, attach);
> > > -		return obj;
> > > -	}
> > >   	get_dma_buf(dma_buf);
> > >   	obj->import_attach = attach;
> > >   	return obj;
> > >   
> > >   out_err:
> > > -	xe_bo_free(bo);
> > > +	xe_bo_put(bo);
> > >   
> > >   	return obj;
> > >   }