Re: FAILED: patch "[PATCH] drm/xe/vm: prevent UAF with asid based lookup" failed to apply to 6.8-stable tree

[Matthew Auld <matthew.auld@intel.com>]

On 24/04/2024 20:03, Lucas De Marchi wrote:
> On Tue, Apr 23, 2024 at 06:07:58AM GMT, gregkh@linuxfoundation.org wrote:
>>
>> The patch below does not apply to the 6.8-stable tree.
>> If someone wants it applied there, or to any other stable or longterm
>> tree, then please email the backport, including the original git commit
>> id to <stable@vger.kernel.org>.
>>
>> To reproduce the conflict and resubmit, you may use the following 
>> commands:
>>
>> git fetch 
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ 
>> linux-6.8.y
>> git checkout FETCH_HEAD
>> git cherry-pick -x ca7c52ac7ad384bcf299d89482c45fec7cd00da9
>> # <resolve conflicts, build, test, etc.>
>> git commit -s
>> git send-email --to '<stable@vger.kernel.org>' --in-reply-to 
>> '2024042358-esteemed-fastball-c2d8@gregkh' --subject-prefix 'PATCH 
>> 6.8.y' HEAD^..
>>
>> Possible dependencies:
>>
>> ca7c52ac7ad3 ("drm/xe/vm: prevent UAF with asid based lookup")
>> 0eb2a18a8fad ("drm/xe: Implement VM snapshot support for BO's and 
>> userptr")
>> be7d51c5b468 ("drm/xe: Add batch buffer addresses to devcoredump")
>> 4376cee62092 ("drm/xe: Print more device information in devcoredump")
>> 98fefec8c381 ("drm/xe: Change devcoredump functions parameters to 
>> xe_sched_job")
> 
> Matt Auld, were we too aggressive saying this should be ported back to
> 6.8?  There's no platform in 6.8 with usm, so maybe we don't really need
> it there.  I don't think we want to bring any of the commits mentioned
> above back to 6.8 really.  If we need this change here, can you prepare
> a modified version with the conflicts resolved for 6.8?

I think it's fine to drop if there is indeed nothing in 6.8 that 
supports usm.

> 
> thanks
> Lucas De Marchi
> 
>>
>> thanks,
>>
>> greg k-h
>>
>> ------------------ original commit in Linus's tree ------------------
>>
>> From ca7c52ac7ad384bcf299d89482c45fec7cd00da9 Mon Sep 17 00:00:00 2001
>> From: Matthew Auld <matthew.auld@intel.com>
>> Date: Fri, 12 Apr 2024 12:31:45 +0100
>> Subject: [PATCH] drm/xe/vm: prevent UAF with asid based lookup
>>
>> The asid is only erased from the xarray when the vm refcount reaches
>> zero, however this leads to potential UAF since the xe_vm_get() only
>> works on a vm with refcount != 0. Since the asid is allocated in the vm
>> create ioctl, rather erase it when closing the vm, prior to dropping the
>> potential last ref. This should also work when user closes driver fd
>> without explicit vm destroy.
>>
>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1594
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: <stable@vger.kernel.org> # v6.8+
>> Reviewed-by: Matthew Brost <matthew.brost@intel.com>
>> Link: 
>> https://patchwork.freedesktop.org/patch/msgid/20240412113144.259426-4-matthew.auld@intel.com
>> (cherry picked from commit 83967c57320d0d01ae512f10e79213f81e4bf594)
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index 62d1ef8867a8..3d4c8f342e21 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -1577,6 +1577,16 @@ void xe_vm_close_and_put(struct xe_vm *vm)
>>         xe->usm.num_vm_in_fault_mode--;
>>     else if (!(vm->flags & XE_VM_FLAG_MIGRATION))
>>         xe->usm.num_vm_in_non_fault_mode--;
>> +
>> +    if (vm->usm.asid) {
>> +        void *lookup;
>> +
>> +        xe_assert(xe, xe->info.has_asid);
>> +        xe_assert(xe, !(vm->flags & XE_VM_FLAG_MIGRATION));
>> +
>> +        lookup = xa_erase(&xe->usm.asid_to_vm, vm->usm.asid);
>> +        xe_assert(xe, lookup == vm);
>> +    }
>>     mutex_unlock(&xe->usm.lock);
>>
>>     for_each_tile(tile, xe, id)
>> @@ -1592,24 +1602,15 @@ static void vm_destroy_work_func(struct 
>> work_struct *w)
>>     struct xe_device *xe = vm->xe;
>>     struct xe_tile *tile;
>>     u8 id;
>> -    void *lookup;
>>
>>     /* xe_vm_close_and_put was not called? */
>>     xe_assert(xe, !vm->size);
>>
>>     mutex_destroy(&vm->snap_mutex);
>>
>> -    if (!(vm->flags & XE_VM_FLAG_MIGRATION)) {
>> +    if (!(vm->flags & XE_VM_FLAG_MIGRATION))
>>         xe_device_mem_access_put(xe);
>>
>> -        if (xe->info.has_asid && vm->usm.asid) {
>> -            mutex_lock(&xe->usm.lock);
>> -            lookup = xa_erase(&xe->usm.asid_to_vm, vm->usm.asid);
>> -            xe_assert(xe, lookup == vm);
>> -            mutex_unlock(&xe->usm.lock);
>> -        }
>> -    }
>> -
>>     for_each_tile(tile, xe, id)
>>         XE_WARN_ON(vm->pt_root[id]);
>>
>>