[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240904123542.2b22a92e@collabora.com>
Date: Wed, 4 Sep 2024 12:35:42 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Mary Guillemard <mary.guillemard@...labora.com>
Cc: linux-kernel@...r.kernel.org, kernel@...labora.com,
stable@...r.kernel.org, Steven Price <steven.price@....com>, Liviu Dudau
<liviu.dudau@....com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, Heiko Stuebner <heiko@...ech.de>,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/panthor: Restrict high priorities on group_create
On Tue, 3 Sep 2024 16:49:55 +0200
Mary Guillemard <mary.guillemard@...labora.com> wrote:
> We were allowing any users to create a high priority group without any
> permission checks. As a result, this was allowing possible denial of
> service.
>
> We now only allow the DRM master or users with the CAP_SYS_NICE
> capability to set higher priorities than PANTHOR_GROUP_PRIORITY_MEDIUM.
>
> As the sole user of that uAPI lives in Mesa and hardcode a value of
> MEDIUM [1], this should be safe to do.
>
> Additionally, as those checks are performed at the ioctl level,
> panthor_group_create now only check for priority level validity.
>
> [1]https://gitlab.freedesktop.org/mesa/mesa/-/blob/f390835074bdf162a63deb0311d1a6de527f9f89/src/gallium/drivers/panfrost/pan_csf.c#L1038
>
> Signed-off-by: Mary Guillemard <mary.guillemard@...labora.com>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Cc: stable@...r.kernel.org
Reviewed-by: Boris Brezillon <boris.brezillon@...labora.com>
> ---
> drivers/gpu/drm/panthor/panthor_drv.c | 23 +++++++++++++++++++++++
> drivers/gpu/drm/panthor/panthor_sched.c | 2 +-
> include/uapi/drm/panthor_drm.h | 6 +++++-
> 3 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index b5e7b919f241..34182f67136c 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -10,6 +10,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
>
> +#include <drm/drm_auth.h>
> #include <drm/drm_debugfs.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_exec.h>
> @@ -996,6 +997,24 @@ static int panthor_ioctl_group_destroy(struct drm_device *ddev, void *data,
> return panthor_group_destroy(pfile, args->group_handle);
> }
>
> +static int group_priority_permit(struct drm_file *file,
> + u8 priority)
> +{
> + /* Ensure that priority is valid */
> + if (priority > PANTHOR_GROUP_PRIORITY_HIGH)
> + return -EINVAL;
> +
> + /* Medium priority and below are always allowed */
> + if (priority <= PANTHOR_GROUP_PRIORITY_MEDIUM)
> + return 0;
> +
> + /* Higher priorities require CAP_SYS_NICE or DRM_MASTER */
> + if (capable(CAP_SYS_NICE) || drm_is_current_master(file))
> + return 0;
> +
> + return -EACCES;
> +}
> +
> static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
> struct drm_file *file)
> {
> @@ -1011,6 +1030,10 @@ static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
> if (ret)
> return ret;
>
> + ret = group_priority_permit(file, args->priority);
> + if (ret)
> + return ret;
> +
> ret = panthor_group_create(pfile, args, queue_args);
> if (ret >= 0) {
> args->group_handle = ret;
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index c426a392b081..91a31b70c037 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3092,7 +3092,7 @@ int panthor_group_create(struct panthor_file *pfile,
> if (group_args->pad)
> return -EINVAL;
>
> - if (group_args->priority > PANTHOR_CSG_PRIORITY_HIGH)
> + if (group_args->priority >= PANTHOR_CSG_PRIORITY_COUNT)
> return -EINVAL;
>
> if ((group_args->compute_core_mask & ~ptdev->gpu_info.shader_present) ||
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index 926b1deb1116..e23a7f9b0eac 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -692,7 +692,11 @@ enum drm_panthor_group_priority {
> /** @PANTHOR_GROUP_PRIORITY_MEDIUM: Medium priority group. */
> PANTHOR_GROUP_PRIORITY_MEDIUM,
>
> - /** @PANTHOR_GROUP_PRIORITY_HIGH: High priority group. */
> + /**
> + * @PANTHOR_GROUP_PRIORITY_HIGH: High priority group.
> + *
> + * Requires CAP_SYS_NICE or DRM_MASTER.
> + */
> PANTHOR_GROUP_PRIORITY_HIGH,
> };
>
>
> base-commit: a15710027afb40c7c1e352902fa5b8c949f021de
Powered by blists - more mailing lists