[PATCH] fix: accel/qaic: qaic_attach_slice_bo_ioctl: success path missing drm_gem_object_put

[PATCH] fix: accel/qaic: qaic_attach_slice_bo_ioctl: success path missing drm_gem_object_put

[WenTao Liang]

drm_gem_object_lookup() acquires a GEM object reference on success. All
  error paths correctly release it via put_bo, but the success path returns
  without calling drm_gem_object_put(obj). Since list_add_tail does not
  transfer ownership, the GEM object reference is permanently leaked on
  each successful call.

Cc: stable@vger.kernel.org
Fixes: 75af0a585af9 ("accel/qaic: Grab ch_lock during QAIC_ATTACH_SLICE_BO")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
 drivers/accel/qaic/qaic_data.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 1e4c579d2725..b17df7bf565d 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1084,6 +1084,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 
 	bo->sliced = true;
 	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
+	drm_gem_object_put(obj);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	mutex_unlock(&bo->lock);
 	kfree(slice_ent);
-- 
2.39.5 (Apple Git-154)

[PATCH] fix: accel/qaic: qaic_attach_slice_bo_ioctl: success path missing drm_gem_object_put

[WenTao Liang]

drm_gem_object_lookup() acquires a GEM object reference on success. All
  error paths correctly release it via put_bo, but the success path returns
  without calling drm_gem_object_put(obj). Since list_add_tail does not
  transfer ownership, the GEM object reference is permanently leaked on
  each successful call.

Cc: stable@vger.kernel.org
Fixes: 75af0a585af9 ("accel/qaic: Grab ch_lock during QAIC_ATTACH_SLICE_BO")
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
 drivers/accel/qaic/qaic_data.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 1e4c579d2725..b17df7bf565d 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1084,6 +1084,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 
 	bo->sliced = true;
 	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
+	drm_gem_object_put(obj);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	mutex_unlock(&bo->lock);
 	kfree(slice_ent);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] fix: accel/qaic: qaic_attach_slice_bo_ioctl: success path missing drm_gem_object_put

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Reference count underflow and Use-After-Free (UAF) due to premature GEM object release.

Pre-existing issues:
- [High] Concurrent list operations on `dbc->bo_lists` are not synchronized, leading to list corruption.
--

commit 9afead81ed2f91d5b21fa630783b8bff00c9912d
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: accel/qaic: qaic_attach_slice_bo_ioctl: success path missing drm_gem_object_put

This commit adds a drm_gem_object_put() call to the success path of
qaic_attach_slice_bo_ioctl(). It attempts to prevent a perceived GEM
object reference leak.

> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index ef0351b6dc9b5..46842d4a0848e 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
> @@ -1084,6 +1084,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
>  
>  	bo->sliced = true;
>  	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);

[Severity: High]
This isn't a bug introduced by this patch, but are the list operations on
dbc->bo_lists safely synchronized?

Looking at qaic_attach_slice_bo_ioctl() and detach_slice_bo(), they modify
this list using list_add_tail() and list_del(). However, the only shared
lock held appears to be srcu_read_lock(&dbc->ch_lock), which allows
concurrent readers. The mutex_lock(&bo->lock) only protects the specific BO.

Could concurrent ioctl calls for different BOs on the same dbc corrupt the
list?

> +	drm_gem_object_put(obj);

[Severity: Critical]
Does adding this drm_gem_object_put() cause a reference count underflow?

The original code appears to intentionally retain this GEM object reference
because list_add_tail() transfers ownership of the BO to dbc->bo_lists.

Later, detach_slice_bo() explicitly drops this list reference:

drivers/accel/qaic/qaic_data.c:detach_slice_bo() {
    ...
	list_del(&bo->bo_list);
	drm_gem_object_put(&bo->base);
    ...
}

If the reference is dropped here during attachment, could the BO be freed
prematurely while still in dbc->bo_lists, leading to a use-after-free
when release_dbc() iterates over the list?

>  	srcu_read_unlock(&dbc->ch_lock, rcu_id);
>  	mutex_unlock(&bo->lock);
>  	kfree(slice_ent);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626114359.32938-1-vulab@iscas.ac.cn?part=1