[PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround

* [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
@ 2019-05-27  8:17 Emil Velikov
  2019-05-27  8:17 ` [PATCH 03/13] drm/etnaviv: drop DRM_AUTH usage from the driver Emil Velikov
                   ` (9 more replies)
  0 siblings, 10 replies; 104+ messages in thread
From: Emil Velikov @ 2019-05-27  8:17 UTC (permalink / raw)
  To: dri-devel
  Cc: David Airlie, emil.l.velikov, amd-gfx, Alex Deucher,
	Christian König

From: Emil Velikov <emil.velikov@collabora.com>

Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the
render node. A seemingly deliberate design decision.

Hence we can drop the DRM_AUTH all together (details in follow-up patch)
yet not all userspace checks if it's authenticated, but instead uses
uncommon assumptions.

After days of digging through git log and testing, only a single (ab)use
was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and
assuming that failure implies lack of authentication.

Affected versions are:
 - the whole 18.2.x series, which is EOL
 - the whole 18.3.x series, which is EOL
 - the 19.0.x series, prior to 19.0.4

Add a special quirk for that case, thus we can drop DRM_AUTH bits as
mentioned earlier.

Since all the affected userspace is EOL, we also add a kconfig option
to disable this quirk.

The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
 drivers/gpu/drm/amd/amdgpu/Kconfig      | 16 ++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++++++++++-
 drivers/gpu/drm/drm_ioctl.c             |  5 +++++
 include/drm/drm_ioctl.h                 | 17 +++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig
index 9221e5489069..da415f445187 100644
--- a/drivers/gpu/drm/amd/amdgpu/Kconfig
+++ b/drivers/gpu/drm/amd/amdgpu/Kconfig
@@ -40,6 +40,22 @@ config DRM_AMDGPU_GART_DEBUGFS
 	  Selecting this option creates a debugfs file to inspect the mapped
 	  pages. Uses more memory for housekeeping, enable only for debugging.
 
+config DRM_AMDGPU_FORCE_AUTH
+	bool "Force authentication check on AMDGPU_INFO ioctl"
+	default y
+	help
+	  There were some version of the Mesa RADV drivers, which relied on
+	  the ioctl failing, if the client is not authenticated.
+
+	  Namely, the following versions are affected:
+	    - the whole 18.2.x series, which is EOL
+	    - the whole 18.3.x series, which is EOL
+	    - the 19.0.x series, prior to 19.0.4
+
+	  Modern distributions, should disable this. That will allow various
+	  other clients to work, that would otherwise require root privileges.
+
+
 source "drivers/gpu/drm/amd/acp/Kconfig"
 source "drivers/gpu/drm/amd/display/Kconfig"
 source "drivers/gpu/drm/amd/amdkfd/Kconfig"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index b17d0545728e..b8076929440b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1214,7 +1214,17 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_WAIT_IDLE, amdgpu_gem_wait_idle_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_CS, amdgpu_cs_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+	/* The DRM_FORCE_AUTH is effectively a workaround for the RADV Mesa driver.
+	 * This is required for Mesa:
+	 *  - the whole 18.2.x series, which is EOL
+	 *  - the whole 18.3.x series, which is EOL
+	 *  - the 19.0.x series, prior to 19.0.4
+	 */
+	DRM_IOCTL_DEF_DRV(AMDGPU_INFO, amdgpu_info_ioctl,
+#if defined(DRM_AMDGPU_FORCE_AUTH)
+		DRM_FORCE_AUTH|
+#endif
+		DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_CS, amdgpu_cs_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_WAIT_FENCES, amdgpu_cs_wait_fences_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2263e3ddd822..9841c0076f02 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -544,6 +544,11 @@ int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 		     drm_is_render_client(file_priv)))
 		return -EACCES;
 
+	/* FORCE_AUTH is only for authenticated or render client */
+	if (unlikely((flags & DRM_FORCE_AUTH) && !drm_is_render_client(file_priv) &&
+		     !file_priv->authenticated))
+		return -EACCES;
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_ioctl_permit);
diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
index fafb6f592c4b..6084ee32043d 100644
--- a/include/drm/drm_ioctl.h
+++ b/include/drm/drm_ioctl.h
@@ -126,6 +126,23 @@ enum drm_ioctl_flags {
 	 * not set DRM_AUTH because they do not require authentication.
 	 */
 	DRM_RENDER_ALLOW	= BIT(5),
+	/**
+	 * @DRM_FORCE_AUTH:
+	 *
+	 * Authentication of the primary node is mandatory. Regardless that the
+	 * user can usually circumvent that by using the render node with exact
+	 * same ioctl.
+	 *
+	 * Note: this is effectively a workaround for AMDGPU AMDGPU_INFO ioctl
+	 * and the RADV Mesa driver. This is required for Mesa:
+	 *  - the whole 18.2.x series, which is EOL
+	 *  - the whole 18.3.x series, which is EOL
+	 *  - the 19.0.x series, prior to 19.0.4
+	 *
+	 * Note: later patch will effectively drop the DRM_AUTH for ioctls
+	 * annotated as DRM_AUTH | DRM_RENDER_ALLOW.
+	 */
+	DRM_FORCE_AUTH          = BIT(6),
 };
 
 /**
-- 
2.21.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 104+ messages in thread