* [PATCH] drm/panthor: Restrict high priorities on group_create
@ 2024-09-03 14:49 Mary Guillemard
2024-09-04 10:35 ` Boris Brezillon
2024-09-05 7:36 ` Boris Brezillon
0 siblings, 2 replies; 3+ messages in thread
From: Mary Guillemard @ 2024-09-03 14:49 UTC (permalink / raw)
To: linux-kernel
Cc: kernel, Mary Guillemard, stable, Boris Brezillon, Steven Price,
Liviu Dudau, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Heiko Stuebner, dri-devel
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@collabora.com>
Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
Cc: stable@vger.kernel.org
---
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
--
2.46.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/panthor: Restrict high priorities on group_create
2024-09-03 14:49 [PATCH] drm/panthor: Restrict high priorities on group_create Mary Guillemard
@ 2024-09-04 10:35 ` Boris Brezillon
2024-09-05 7:36 ` Boris Brezillon
1 sibling, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2024-09-04 10:35 UTC (permalink / raw)
To: Mary Guillemard
Cc: linux-kernel, kernel, stable, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, Heiko Stuebner, dri-devel
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/f390835074bdf162a63deb0311d1a6de527f9f89/src/gallium/drivers/panfrost/pan_csf.c#L1038
>
> 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.
> + */
> PANTHOR_GROUP_PRIORITY_HIGH,
> };
>
>
> base-commit: a15710027afb40c7c1e352902fa5b8c949f021de
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/panthor: Restrict high priorities on group_create
2024-09-03 14:49 [PATCH] drm/panthor: Restrict high priorities on group_create Mary Guillemard
2024-09-04 10:35 ` Boris Brezillon
@ 2024-09-05 7:36 ` Boris Brezillon
1 sibling, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2024-09-05 7:36 UTC (permalink / raw)
To: Mary Guillemard
Cc: linux-kernel, kernel, stable, Steven Price, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, Heiko Stuebner, dri-devel
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/f390835074bdf162a63deb0311d1a6de527f9f89/src/gallium/drivers/panfrost/pan_csf.c#L1038
>
> Signed-off-by: Mary Guillemard <mary.guillemard@collabora.com>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Cc: stable@vger.kernel.org
Queued to drm-misc-fixes.
Thanks!
> ---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-05 7:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-03 14:49 [PATCH] drm/panthor: Restrict high priorities on group_create Mary Guillemard
2024-09-04 10:35 ` Boris Brezillon
2024-09-05 7:36 ` Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).