Currently we just leave it uninitialised, which at first looks harmless, however we also don't zero out the pfn array, and with pfn_flags_mask the idea is to be able set individual flags for a given range of pfn or completely ignore them, outside of default_flags. So here we end up with pfn[i] & pfn_flags_mask, and if both are uninitialised we might get back an unexpected flags value, like asking for read only with default_flags, but getting back write on top, leading to potentially bogus behaviour.
To fix this ensure we zero the pfn_flags_mask, such that hmm only considers the default_flags and not also the initial pfn[i] value.
Fixes: 81e058a3e7fd ("drm/xe: Introduce helper to populate userptr") Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Matthew Brost matthew.brost@intel.com Cc: Thomas Hellström thomas.hellstrom@intel.com Cc: stable@vger.kernel.org # v6.10+ --- drivers/gpu/drm/xe/xe_hmm.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xe/xe_hmm.c b/drivers/gpu/drm/xe/xe_hmm.c index 089834467880..8c3cd65fa4b3 100644 --- a/drivers/gpu/drm/xe/xe_hmm.c +++ b/drivers/gpu/drm/xe/xe_hmm.c @@ -206,6 +206,7 @@ int xe_hmm_userptr_populate_range(struct xe_userptr_vma *uvma, goto free_pfns; }
+ hmm_range.pfn_flags_mask = 0; hmm_range.default_flags = flags; hmm_range.hmm_pfns = pfns; hmm_range.notifier = &userptr->notifier;
On Wed, 2025-02-26 at 17:17 +0000, Matthew Auld wrote:
Currently we just leave it uninitialised, which at first looks harmless, however we also don't zero out the pfn array, and with pfn_flags_mask the idea is to be able set individual flags for a given range of pfn or completely ignore them, outside of default_flags. So here we end up with pfn[i] & pfn_flags_mask, and if both are uninitialised we might get back an unexpected flags value, like asking for read only with default_flags, but getting back write on top, leading to potentially bogus behaviour.
To fix this ensure we zero the pfn_flags_mask, such that hmm only considers the default_flags and not also the initial pfn[i] value.
Fixes: 81e058a3e7fd ("drm/xe: Introduce helper to populate userptr") Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Matthew Brost matthew.brost@intel.com Cc: Thomas Hellström thomas.hellstrom@intel.com Cc: stable@vger.kernel.org # v6.10+
drivers/gpu/drm/xe/xe_hmm.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xe/xe_hmm.c b/drivers/gpu/drm/xe/xe_hmm.c index 089834467880..8c3cd65fa4b3 100644 --- a/drivers/gpu/drm/xe/xe_hmm.c +++ b/drivers/gpu/drm/xe/xe_hmm.c @@ -206,6 +206,7 @@ int xe_hmm_userptr_populate_range(struct xe_userptr_vma *uvma, goto free_pfns; }
- hmm_range.pfn_flags_mask = 0;
hmm_range.default_flags = flags; hmm_range.hmm_pfns = pfns; hmm_range.notifier = &userptr->notifier;
Is there a chance we can modify the function to have an initializer for hmm_range
struct hmm_range hmm_range = {...
That'd also make adding fields to struct hmm_range less fragile.
Either way, Reviewed-by: Thomas Hellström thomas.hellstrom@linux.intel.com
-----Original Message----- From: Intel-xe intel-xe-bounces@lists.freedesktop.org On Behalf Of Matthew Auld Sent: Wednesday, February 26, 2025 10:47 PM To: intel-xe@lists.freedesktop.org Cc: Brost, Matthew matthew.brost@intel.com; Hellstrom, Thomas thomas.hellstrom@intel.com; stable@vger.kernel.org Subject: [PATCH] drm/xe/userptr: properly setup pfn_flags_mask
Currently we just leave it uninitialised, which at first looks harmless, however we also don't zero out the pfn array, and with pfn_flags_mask the idea is to be able set individual flags for a given range of pfn or completely ignore them, outside of default_flags. So here we end up with pfn[i] & pfn_flags_mask, and if both are uninitialised we might get back an unexpected flags value, like asking for read only with default_flags, but getting back write on top, leading to potentially bogus behaviour.
To fix this ensure we zero the pfn_flags_mask, such that hmm only considers the default_flags and not also the initial pfn[i] value.
Fixes: 81e058a3e7fd ("drm/xe: Introduce helper to populate userptr") Signed-off-by: Matthew Auld matthew.auld@intel.com Cc: Matthew Brost matthew.brost@intel.com Cc: Thomas Hellström thomas.hellstrom@intel.com Cc: stable@vger.kernel.org # v6.10+
drivers/gpu/drm/xe/xe_hmm.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xe/xe_hmm.c b/drivers/gpu/drm/xe/xe_hmm.c index 089834467880..8c3cd65fa4b3 100644 --- a/drivers/gpu/drm/xe/xe_hmm.c +++ b/drivers/gpu/drm/xe/xe_hmm.c @@ -206,6 +206,7 @@ int xe_hmm_userptr_populate_range(struct xe_userptr_vma *uvma, goto free_pfns; }
- hmm_range.pfn_flags_mask = 0;
Looks correct to me, Reviewed-by: Tejas Upadhyay tejas.upadhyay@intel.com
Tejas
hmm_range.default_flags = flags; hmm_range.hmm_pfns = pfns; hmm_range.notifier = &userptr->notifier; -- 2.48.1
linux-stable-mirror@lists.linaro.org
-
Hellstrom, Thomas -
Matthew Auld -
Upadhyay, Tejas