* [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF
@ 2024-09-25 7:14 Matthew Auld
2024-09-25 9:24 ` Nirmoy Das
2024-09-25 16:33 ` Matthew Brost
0 siblings, 2 replies; 3+ messages in thread
From: Matthew Auld @ 2024-09-25 7:14 UTC (permalink / raw)
To: intel-xe; +Cc: Matthew Brost, stable
Evil user can guess the next id of the vm before the ioctl completes and
then call vm destroy ioctl to trigger UAF since create ioctl is still
referencing the same vm. Move the xa_alloc all the way to the end to
prevent this.
v2:
- Rebase
Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: <stable@vger.kernel.org> # v6.8+
---
drivers/gpu/drm/xe/xe_vm.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 31fe31db3fdc..ce9dca4d4e87 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1765,10 +1765,6 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
if (IS_ERR(vm))
return PTR_ERR(vm);
- err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
- if (err)
- goto err_close_and_put;
-
if (xe->info.has_asid) {
down_write(&xe->usm.lock);
err = xa_alloc_cyclic(&xe->usm.asid_to_vm, &asid, vm,
@@ -1776,12 +1772,11 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
&xe->usm.next_asid, GFP_KERNEL);
up_write(&xe->usm.lock);
if (err < 0)
- goto err_free_id;
+ goto err_close_and_put;
vm->usm.asid = asid;
}
- args->vm_id = id;
vm->xef = xe_file_get(xef);
/* Record BO memory for VM pagetable created against client */
@@ -1794,10 +1789,15 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
args->reserved[0] = xe_bo_main_addr(vm->pt_root[0]->bo, XE_PAGE_SIZE);
#endif
+ /* user id alloc must always be last in ioctl to prevent UAF */
+ err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
+ if (err)
+ goto err_close_and_put;
+
+ args->vm_id = id;
+
return 0;
-err_free_id:
- xa_erase(&xef->vm.xa, id);
err_close_and_put:
xe_vm_close_and_put(vm);
--
2.46.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF
2024-09-25 7:14 [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF Matthew Auld
@ 2024-09-25 9:24 ` Nirmoy Das
2024-09-25 16:33 ` Matthew Brost
1 sibling, 0 replies; 3+ messages in thread
From: Nirmoy Das @ 2024-09-25 9:24 UTC (permalink / raw)
To: Matthew Auld, intel-xe; +Cc: Matthew Brost, stable
On 9/25/2024 9:14 AM, Matthew Auld wrote:
> Evil user can guess the next id of the vm before the ioctl completes and
> then call vm destroy ioctl to trigger UAF since create ioctl is still
> referencing the same vm. Move the xa_alloc all the way to the end to
> prevent this.
>
> v2:
> - Rebase
>
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: <stable@vger.kernel.org> # v6.8+
Interesting find, LGTM:
Reviewed-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
> drivers/gpu/drm/xe/xe_vm.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 31fe31db3fdc..ce9dca4d4e87 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1765,10 +1765,6 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> if (IS_ERR(vm))
> return PTR_ERR(vm);
>
> - err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
> - if (err)
> - goto err_close_and_put;
> -
> if (xe->info.has_asid) {
> down_write(&xe->usm.lock);
> err = xa_alloc_cyclic(&xe->usm.asid_to_vm, &asid, vm,
> @@ -1776,12 +1772,11 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> &xe->usm.next_asid, GFP_KERNEL);
> up_write(&xe->usm.lock);
> if (err < 0)
> - goto err_free_id;
> + goto err_close_and_put;
>
> vm->usm.asid = asid;
> }
>
> - args->vm_id = id;
> vm->xef = xe_file_get(xef);
>
> /* Record BO memory for VM pagetable created against client */
> @@ -1794,10 +1789,15 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> args->reserved[0] = xe_bo_main_addr(vm->pt_root[0]->bo, XE_PAGE_SIZE);
> #endif
>
> + /* user id alloc must always be last in ioctl to prevent UAF */
> + err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
> + if (err)
> + goto err_close_and_put;
> +
> + args->vm_id = id;
> +
> return 0;
>
> -err_free_id:
> - xa_erase(&xef->vm.xa, id);
> err_close_and_put:
> xe_vm_close_and_put(vm);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF
2024-09-25 7:14 [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF Matthew Auld
2024-09-25 9:24 ` Nirmoy Das
@ 2024-09-25 16:33 ` Matthew Brost
1 sibling, 0 replies; 3+ messages in thread
From: Matthew Brost @ 2024-09-25 16:33 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-xe, stable
On Wed, Sep 25, 2024 at 08:14:27AM +0100, Matthew Auld wrote:
> Evil user can guess the next id of the vm before the ioctl completes and
> then call vm destroy ioctl to trigger UAF since create ioctl is still
> referencing the same vm. Move the xa_alloc all the way to the end to
> prevent this.
>
> v2:
> - Rebase
>
> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> Cc: <stable@vger.kernel.org> # v6.8+
> ---
> drivers/gpu/drm/xe/xe_vm.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 31fe31db3fdc..ce9dca4d4e87 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -1765,10 +1765,6 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> if (IS_ERR(vm))
> return PTR_ERR(vm);
>
> - err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
> - if (err)
> - goto err_close_and_put;
> -
> if (xe->info.has_asid) {
> down_write(&xe->usm.lock);
> err = xa_alloc_cyclic(&xe->usm.asid_to_vm, &asid, vm,
> @@ -1776,12 +1772,11 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> &xe->usm.next_asid, GFP_KERNEL);
> up_write(&xe->usm.lock);
> if (err < 0)
> - goto err_free_id;
> + goto err_close_and_put;
>
> vm->usm.asid = asid;
> }
>
> - args->vm_id = id;
> vm->xef = xe_file_get(xef);
>
> /* Record BO memory for VM pagetable created against client */
> @@ -1794,10 +1789,15 @@ int xe_vm_create_ioctl(struct drm_device *dev, void *data,
> args->reserved[0] = xe_bo_main_addr(vm->pt_root[0]->bo, XE_PAGE_SIZE);
> #endif
>
> + /* user id alloc must always be last in ioctl to prevent UAF */
> + err = xa_alloc(&xef->vm.xa, &id, vm, xa_limit_32b, GFP_KERNEL);
> + if (err)
> + goto err_close_and_put;
> +
> + args->vm_id = id;
> +
> return 0;
>
> -err_free_id:
> - xa_erase(&xef->vm.xa, id);
> err_close_and_put:
> xe_vm_close_and_put(vm);
>
> --
> 2.46.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-25 16:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-25 7:14 [PATCH v2 1/2] drm/xe/vm: move xa_alloc to prevent UAF Matthew Auld
2024-09-25 9:24 ` Nirmoy Das
2024-09-25 16:33 ` Matthew Brost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox