All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arvind Yadav" <arvind.yadav@intel.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [RFC v3 2/7] drm/xe/vm: Preserve cpu_autoreset_active across GPUVA operations
Date: Thu, 04 Jun 2026 10:07:21 +0000	[thread overview]
Message-ID: <20260604100721.7ECEA1F00899@smtp.kernel.org> (raw)
In-Reply-To: <20260604095012.367231-3-arvind.yadav@intel.com>

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 <arvind.yadav@intel.com>

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/20260604095012.367231-1-arvind.yadav@intel.com?part=2

  reply	other threads:[~2026-06-04 10:07 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04  9:50 [RFC v3 0/7] drm/xe/svm: Add MMU notifier-based madvise autoreset on munmap Arvind Yadav
2026-06-04  9:50 ` [RFC v3 1/7] drm/xe/vm: Track CPU_AUTORESET state in xe_vma Arvind Yadav
2026-06-04  9:50 ` [RFC v3 2/7] drm/xe/vm: Preserve cpu_autoreset_active across GPUVA operations Arvind Yadav
2026-06-04 10:07   ` sashiko-bot [this message]
2026-06-04  9:50 ` [RFC v3 3/7] drm/xe/svm: Clear CPU_AUTORESET_ACTIVE on first GPU fault Arvind Yadav
2026-06-04  9:50 ` [RFC v3 4/7] drm/xe/vm: Add madvise autoreset notifier worker Arvind Yadav
2026-06-04 10:14   ` sashiko-bot
2026-06-04  9:50 ` [RFC v3 5/7] drm/xe/vm: Disable madvise notifier on GPU touch Arvind Yadav
2026-06-04 10:03   ` sashiko-bot
2026-06-04  9:50 ` [RFC v3 6/7] drm/xe/vm: Wire MADVISE_AUTORESET notifiers into VM lifecycle Arvind Yadav
2026-06-04 10:05   ` sashiko-bot
2026-06-04  9:50 ` [RFC v3 7/7] drm/xe/svm: Correct memory attribute reset for partial unmap Arvind Yadav
2026-06-04 10:12   ` sashiko-bot
2026-06-04 10:32 ` ✗ CI.checkpatch: warning for drm/xe/svm: Add MMU notifier-based madvise autoreset on munmap (rev3) Patchwork
2026-06-04 10:34 ` ✓ CI.KUnit: success " Patchwork
2026-06-04 11:35 ` ✓ Xe.CI.BAT: " Patchwork
2026-06-05  0:02 ` ✗ Xe.CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260604100721.7ECEA1F00899@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=arvind.yadav@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.