sna: Always validate userptr upon creation

Since not all memory ranges can be mapped by userptr, in particular those
passed by XShmAttachFD, we need to validate the userptr before use. We
would ideally want to continue to lazily populate the pages as often the
userptr is created but never used, but preventing an EFAULT later is
more important.

In https://patchwork.freedesktop.org/series/33449/ we provided a more
efficient method for probing the userptr on construction while
preserving the lazy population of gup-pages. For now, always follow
userptr with set-domain.

Reported-by: Jinoh Kang <jinoh.kang.kr@gmail.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

1 files changed, 34 insertions, 24 deletions

diff --git a/src/sna/kgem.c b/src/sna/kgem.c
index 2e21bc11..7b645da8 100644
--- a/src/sna/kgem.c
+++ b/src/sna/kgem.c
@@ -520,7 +520,7 @@ static bool gem_set_caching(int fd, uint32_t handle, int caching)
 	return do_ioctl(fd, LOCAL_IOCTL_I915_GEM_SET_CACHING, &arg) == 0;
 }
 
-static uint32_t gem_userptr(int fd, void *ptr, int size, int read_only)
+static uint32_t gem_userptr(int fd, void *ptr, size_t size, int read_only)
 {
 	struct local_i915_gem_userptr arg;
 
@@ -7031,6 +7031,30 @@ uint32_t kgem_bo_flink(struct kgem *kgem, struct kgem_bo *bo)
 	return flink.name;
 }
 
+static bool probe(struct kgem *kgem, uint32_t handle)
+{
+	struct drm_i915_gem_set_domain arg = {
+		.handle = handle,
+		.read_domains = I915_GEM_DOMAIN_CPU,
+	};
+
+	return do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &arg) == 0;
+}
+
+static uint32_t probe_userptr(struct kgem *kgem,
+			      void *ptr, size_t size, int read_only)
+{
+	uint32_t handle;
+
+	handle = gem_userptr(kgem->fd, ptr, size, read_only);
+	if (handle && !probe(kgem, handle)) {
+		gem_close(kgem->fd, handle);
+		handle = 0;
+	}
+
+	return handle;
+}
+
 struct kgem_bo *kgem_create_map(struct kgem *kgem,
 				void *ptr, uint32_t size,
 				bool read_only)
@@ -7053,30 +7077,16 @@ struct kgem_bo *kgem_create_map(struct kgem *kgem,
 	last_page &= ~(uintptr_t)(PAGE_SIZE-1);
 	assert(last_page > first_page);
 
-	handle = gem_userptr(kgem->fd,
-			     (void *)first_page, last_page-first_page,
-			     read_only);
+	handle = probe_userptr(kgem,
+			       (void *)first_page, last_page-first_page,
+			       read_only);
+	if (handle == 0 && read_only && kgem->has_wc_mmap)
+		handle = probe_userptr(kgem,
+				       (void *)first_page, last_page-first_page,
+				       false);
 	if (handle == 0) {
-		if (read_only && kgem->has_wc_mmap) {
-			struct drm_i915_gem_set_domain set_domain;
-
-			handle = gem_userptr(kgem->fd,
-					     (void *)first_page, last_page-first_page,
-					     false);
-
-			VG_CLEAR(set_domain);
-			set_domain.handle = handle;
-			set_domain.read_domains = I915_GEM_DOMAIN_GTT;
-			set_domain.write_domain = 0;
-			if (do_ioctl(kgem->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)) {
-				gem_close(kgem->fd, handle);
-				handle = 0;
-			}
-		}
-		if (handle == 0) {
-			DBG(("%s: import failed, errno=%d\n", __FUNCTION__, errno));
-			return NULL;
-		}
+		DBG(("%s: import failed, errno=%d\n", __FUNCTION__, errno));
+		return NULL;
 	}
 
 	bo = __kgem_bo_alloc(handle, (last_page - first_page) / PAGE_SIZE);