* [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs
@ 2026-11-06 19:44 Youssef Samir
2026-11-06 19:52 ` sashiko-bot
2026-11-09 17:23 ` Jeff Hugo
0 siblings, 2 replies; 3+ messages in thread
From: Youssef Samir @ 2026-11-06 19:44 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel
send_bo_list_to_device() does not validate if the BOs the user passes
through DRM_QAIC_EXECUTE_BO and DRM_QAIC_PARTIAL_EXECUTE_BO IOCTLs are
attached to the DBC provided. This allows the user to slice and attach
BOs to one DBC, and execute them over another DBC.
The same case applies for qaic_wait_bo_ioctl() as well as it neither
checks if the BO is sliced, nor if it attached to the passed DBC.
Fixes: ff13be830333 ("accel/qaic: Add datapath")
Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
---
drivers/accel/qaic/qaic_data.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index ee35e13091b3..be3ceb8c51a0 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1256,6 +1256,11 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
goto unlock_bo;
}
+ if (bo->dbc->id != dbc->id) {
+ ret = -EINVAL;
+ goto unlock_bo;
+ }
+
if (is_partial && pexec[i].resize > bo->base.size) {
ret = -EINVAL;
goto unlock_bo;
@@ -1769,6 +1774,17 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
}
bo = to_qaic_bo(obj);
+
+ if (!bo->sliced) {
+ ret = -EINVAL;
+ goto put_obj;
+ }
+
+ if (bo->dbc->id != dbc->id) {
+ ret = -EINVAL;
+ goto put_obj;
+ }
+
timeout = args->timeout ? args->timeout : wait_exec_default_timeout_ms;
timeout = msecs_to_jiffies(timeout);
ret = wait_for_completion_interruptible_timeout(&bo->xfer_done, timeout);
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs
2026-11-06 19:44 [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs Youssef Samir
@ 2026-11-06 19:52 ` sashiko-bot
2026-11-09 17:23 ` Jeff Hugo
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-06 19:52 UTC (permalink / raw)
To: Youssef Samir; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] accel/qaic: unlocked bo->dbc dereference in
qaic_wait_bo_ioctl()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] accel/qaic: missing completion signal in
send_bo_list_to_device() error path [1]
--
commit f3a92ee30bd740f37f373a9b22ee385311742a39
Author: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
accel/qaic: Harden checks in BO execute and wait IOCTLs
This commit adds checks in send_bo_list_to_device() and
qaic_wait_bo_ioctl() to verify that BOs are sliced and attached to the
specified DBC.
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index ee35e13091b30..be3ceb8c51a09 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
> @@ -1256,6 +1256,11 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
> goto unlock_bo;
> }
>
> + if (bo->dbc->id != dbc->id) {
> + ret = -EINVAL;
> + goto unlock_bo;
> + }
> +
> if (is_partial && pexec[i].resize > bo->base.size) {
> ret = -EINVAL;
> goto unlock_bo;
[Severity: Medium]
This problem wasn't introduced by this patch, but can failing on a later BO
in the batch leave waiters on earlier BOs hung indefinitely [1]?
For each BO processed before the failure, send_bo_list_to_device() calls
reinit_completion(&bo->xfer_done) and adds the BO to dbc->xfer_list. When a
subsequent BO jumps to unlock_bo and failed_to_send_bo, the rollback loop
removes those earlier BOs from dbc->xfer_list without calling
complete_all(&bo->xfer_done):
drivers/accel/qaic/qaic_data.c:send_bo_list_to_device() {
...
failed_to_send_bo:
if (likely(obj))
drm_gem_object_put(obj);
for (j = 0; j < i; j++) {
spin_lock_irqsave(&dbc->xfer_lock, flags);
bo = list_last_entry(&dbc->xfer_list, struct qaic_bo, xfer_list);
obj = &bo->base;
list_del_init(&bo->xfer_list);
spin_unlock_irqrestore(&dbc->xfer_lock, flags);
dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
drm_gem_object_put(obj);
}
}
Because the rolled-back BOs are no longer on dbc->xfer_list, the interrupt
handler will never complete them, so any thread waiting in
qaic_wait_bo_ioctl() will block until timeout.
> @@ -1769,6 +1774,17 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
> }
>
> bo = to_qaic_bo(obj);
> +
> + if (!bo->sliced) {
> + ret = -EINVAL;
> + goto put_obj;
> + }
> +
> + if (bo->dbc->id != dbc->id) {
> + ret = -EINVAL;
> + goto put_obj;
> + }
> +
> timeout = args->timeout ? args->timeout : wait_exec_default_timeout_ms;
> timeout = msecs_to_jiffies(timeout);
> ret = wait_for_completion_interruptible_timeout(&bo->xfer_done, timeout);
[Severity: High]
Can this dereference a NULL bo->dbc when racing with
qaic_detach_slice_bo_ioctl()?
bo->sliced and bo->dbc are protected by bo->lock, which qaic_wait_bo_ioctl()
does not hold. A concurrent detach clears bo->dbc in qaic_unprepare_bo()
before clearing bo->sliced in qaic_init_bo():
CPU0 CPU1
qaic_wait_bo_ioctl() qaic_detach_slice_bo_ioctl()
mutex_lock(&bo->lock)
detach_slice_bo()
qaic_unprepare_bo()
bo->dbc = NULL
if (!bo->sliced)
if (bo->dbc->id != dbc->id) <-- NULL pointer dereference
qaic_init_bo()
bo->sliced = false
Acquiring bo->lock around the bo->sliced and bo->dbc->id checks prevents the
race.
[1] https://sashiko.dev/#/bug/linux-3d10e194-452c-43b5-a7b5-0f068986229f
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106194434.4062205-1-youssef.abdulrahman@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs
2026-11-06 19:44 [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs Youssef Samir
2026-11-06 19:52 ` sashiko-bot
@ 2026-11-09 17:23 ` Jeff Hugo
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Hugo @ 2026-11-09 17:23 UTC (permalink / raw)
To: Youssef Samir, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel
On 11/6/2026 12:44 PM, Youssef Samir wrote:
> send_bo_list_to_device() does not validate if the BOs the user passes
> through DRM_QAIC_EXECUTE_BO and DRM_QAIC_PARTIAL_EXECUTE_BO IOCTLs are
> attached to the DBC provided. This allows the user to slice and attach
> BOs to one DBC, and execute them over another DBC.
>
> The same case applies for qaic_wait_bo_ioctl() as well as it neither
> checks if the BO is sliced, nor if it attached to the passed DBC.
>
> Fixes: ff13be830333 ("accel/qaic: Add datapath")
> Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-09 17:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 19:44 [PATCH] accel/qaic: Harden checks in BO execute and wait IOCTLs Youssef Samir
2026-11-06 19:52 ` sashiko-bot
2026-11-09 17:23 ` Jeff Hugo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.