[PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
Daniel Vetter
daniel at ffwll.ch
Mon May 27 13:11:43 UTC 2019
On Mon, May 27, 2019 at 09:17:29AM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at 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.
Maybe correction here: Id does not care about authentication at all. It
wants to figure out whether it has access to modeset resources, which is
something entirely different, but happened to match in amdgpu's case.
> 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
Hm I think I'm blind, I'm not seeing the fix merged to mesa master. Still
there on gitlab:
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/radv_device.c#L291
What am I missing?
> 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 at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: amd-gfx at lists.freedesktop.org
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
Aside from the nits I think reasonable approach.
-Daniel
> ---
> 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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the amd-gfx
mailing list