Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing CAP_SYS_NICE or DRM_MASTER check for elevated job priorities (REALTIME / FOCUS), allowing unprivileged users to monopolize the NPU. -- commit 57cf0d7005a7761ad6193ea568802164b7fac6db Author: Andrzej Kacprowski <[email protected]> accel/ivpu: Add support for getting and setting command queue priority This commit adds a new DRM_IVPU_PARAM_CMDQ_PRIORITY parameter to query and update the scheduling priority of an existing command queue. The priority can be changed on a queue that already has pending jobs, propagating the change to firmware in hardware scheduling mode. > diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c > index ebb2c865b09a0..b3de5dd29d1e8 100644 > --- a/drivers/accel/ivpu/ivpu_job.c > +++ b/drivers/accel/ivpu/ivpu_job.c [ ... ] > @@ -1127,6 +1132,86 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, > void *data, struct drm_file > return ret; > } > > +static int ivpu_cmdq_priority_args_check(struct ivpu_device *vdev, u32 > cmdq_id, u64 priority) > +{ > + if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_CMDQ_SET_PRIORITY)) { > + ivpu_dbg(vdev, IOCTL, "Command queue priority not supported\n"); > + return -ENODEV; > + } > + > + if (cmdq_id < IVPU_CMDQ_MIN_ID || cmdq_id > IVPU_CMDQ_MAX_ID) { > + ivpu_dbg(vdev, IOCTL, "Invalid command queue ID %u\n", cmdq_id); > + return -EINVAL; > + } > + > + if (priority > DRM_IVPU_JOB_PRIORITY_REALTIME) > + return -EINVAL; [Severity: High] This is a pre-existing issue, but is there a missing capability check for elevated priorities here? An unprivileged user who opens the NPU device can call the IVPU_SET_PARAM ioctl with DRM_IVPU_PARAM_CMDQ_PRIORITY and set the priority to DRM_IVPU_JOB_PRIORITY_REALTIME. Without a CAP_SYS_NICE or DRM_MASTER check, could a malicious or buggy application elevate its command queue priority to monopolize the hardware and starve other processes? I notice this same behavior appears to be pre-existing in ivpu_cmdq_create_ioctl() as well, where queues can be created with realtime priority by any user. Should these priority levels be restricted? > + > + return 0; > +} > + > +int ivpu_cmdq_get_priority(struct ivpu_file_priv *file_priv, u32 cmdq_id, > u64 *priority) > +{ [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
