On Tue, 3 Sep 2024 16:49:55 +0200 Mary Guillemard mary.guillemard@collabora.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/f390835074bdf162a63deb0311d1...
Signed-off-by: Mary Guillemard mary.guillemard@collabora.com Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block") Cc: stable@vger.kernel.org
Reviewed-by: Boris Brezillon boris.brezillon@collabora.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.*/};
base-commit: a15710027afb40c7c1e352902fa5b8c949f021de