On Tue, 3 Sep 2024 16:49:55 +0200 Mary Guillemard <mary.guillem...@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/f390835074bdf162a63deb0311d1a6de527f9f89/src/gallium/drivers/panfrost/pan_csf.c#L1038 > > Signed-off-by: Mary Guillemard <mary.guillem...@collabora.com> > Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block") > Cc: sta...@vger.kernel.org Reviewed-by: Boris Brezillon <boris.brezil...@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. > + */ > PANTHOR_GROUP_PRIORITY_HIGH, > }; > > > base-commit: a15710027afb40c7c1e352902fa5b8c949f021de
