Am 27.05.19 um 10:17 schrieb Emil Velikov:
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
Well you could have saved your time, cause this is still a NAK.
For the record: I strongly think that we don't want to expose any render functionality on the primary node.
To even go a step further I would say that at least on AMD hardware we should completely disable DRI2 for one of the next generations.
As a follow up I would then completely disallow the DRM authentication for amdgpu, so that the command submission interface on the primary node can only be used by the display server.
Regards, Christian.
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 onthe 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.4Modern distributions, should disable this. That will allow variousother 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),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), };
/**