Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Merging adjacent VMAs during CPU mirror pagefaults silently clobbers creation flags (e.g., READ_ONLY, MADV_AUTORESET), permanently altering memory properties and potentially leading to privilege escalation. -- commit 1d97ca671d925d0481b9d913f989dc8216596164 Author: Arvind Yadav <[email protected]> drm/xe/vm: Preserve cpu_autoreset_active across GPUVA operations GPUVA split and remap operations rebuild VMAs from create flags. This commit explicitly forwards XE_VMA_CPU_AUTORESET_ACTIVE through the MAP/REMAP pipeline so xe_vma_create() can restore cpu_autoreset_active. It also relaxes the UNMAP attribute warning for MADVISE_AUTORESET VMAs. > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c > --- a/drivers/gpu/drm/xe/xe_vm.c > +++ b/drivers/gpu/drm/xe/xe_vm.c [ ... ] > @@ -4728,19 +4752,24 @@ static int xe_vm_alloc_vma(struct xe_vm *vm, > if (!is_madvise) { > if (__op->op == DRM_GPUVA_OP_UNMAP) { > vma = gpuva_to_vma(op->base.unmap.va); > - XE_WARN_ON(!xe_vma_has_default_mem_attrs(vma)); > + /* AUTORESET VMAs may carry madvise-managed > attrs. */ > + XE_WARN_ON(!xe_vma_has_default_mem_attrs(vma) && > + !(vma->gpuva.flags & > XE_VMA_MADV_AUTORESET)); > default_pat = vma->attr.default_pat_index; > - vma_flags = vma->gpuva.flags; > + vma_flags = xe_vma_effective_create_flags(vma); > } > > if (__op->op == DRM_GPUVA_OP_REMAP) { > vma = gpuva_to_vma(op->base.remap.unmap->va); > default_pat = vma->attr.default_pat_index; > - vma_flags = vma->gpuva.flags; > + vma_flags = xe_vma_effective_create_flags(vma); > } > > if (__op->op == DRM_GPUVA_OP_MAP) { > op->map.vma_flags |= vma_flags & > XE_VMA_CREATE_MASK; [Severity: Critical] This is a pre-existing issue, but does this loop silently clobber creation flags like XE_VMA_READ_ONLY when merging adjacent VMAs during CPU mirror pagefaults? When xe_svm_handle_pagefault() attempts to expand a VMA mapping, it calls xe_vm_find_cpu_addr_mirror_vma_range(). That function coalesces adjacent VMAs using is_cpu_addr_vma_with_default_attr(): drivers/gpu/drm/xe/xe_vm.c:is_cpu_addr_vma_with_default_attr() { return vma && xe_vma_is_cpu_addr_mirror(vma) && xe_vma_has_default_mem_attrs(vma); } This check verifies the system allocator flag and default memory attributes, but it doesn't ensure that vma->gpuva.flags (such as XE_VMA_READ_ONLY or XE_VMA_DUMPABLE) match between the adjacent VMAs. As a result, xe_vm_alloc_vma() creates a single MAP operation for the merged range. In the drm_gpuva_for_each_op() loop here, the local vma_flags variable is continuously overwritten by each UNMAP and REMAP operation. When the final MAP operation processes this line, it inherits flags exclusively from the last processed VMA. If a read-write VMA is merged with an adjacent read-only VMA, is the XE_VMA_READ_ONLY flag completely lost for the coalesced region, potentially granting writable access to a mapping originally requested as read-only? > + /* Forward pipeline-only state. */ > + if (vma_flags & XE_VMA_CPU_AUTORESET_ACTIVE) > + op->map.vma_flags |= > XE_VMA_CPU_AUTORESET_ACTIVE; > op->map.pat_index = default_pat; > } > } else { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
