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 <[email protected]> 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/[email protected]?part=1
