Hi,
On 20. 01. 26, 11:54, Christian König wrote:
Some driver use fence->ops to test if a fence was initialized or not. The problem is that this utilizes internal behavior of the dma_fence implementation.
So better abstract that into a function.
v2: use a flag instead of testing fence->ops, rename the function, move to the beginning of the patch set.
...
--- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -146,7 +146,7 @@ qxl_release_free(struct qxl_device *qdev, idr_remove(&qdev->release_idr, release->id); spin_unlock(&qdev->release_idr_lock);
- if (release->base.ops) {
- if (dma_fence_was_initialized(&release->base)) { WARN_ON(list_empty(&release->bos)); qxl_release_free_list(release);
This likely breaks qxl: https://bugzilla.suse.com/show_bug.cgi?id=1271081
refcount_t: underflow; use-after-free. WARNING: lib/refcount.c:28 at refcount_warn_saturate+0x59/0x90,
CPU#0: kworker/0:0/1534
Modules linked in: af_packet nft_fib_inet ... CPU: 0 UID: 0 PID: 1534 Comm: kworker/0:0 Not tainted 7.1.3-1-default
#1 PREEMPT(full) openSUSE Tumbleweed b041a6527f6e58424f4cd3de0fade8d408b378fd
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
rel-1.16.3-2-gc13ff2cd-prebuilt.qemu.org 04/01/2014
Workqueue: events qxl_gc_work [qxl] RIP: 0010:refcount_warn_saturate+0x59/0x90 Code: 44 48 8d 3d a9 72 b7 01 67 48 0f b9 3a c3 cc cc cc cc 48 8d 3d
a8 72 b7 01 67 48 0f b9 3a c3 cc cc cc cc 48 8d 3d a7 72 b7 01 <67> 48 0f b9 3a c3 cc cc cc cc 48 8d 3d a6 72 b7 01 67 48 0f b9 3a
RSP: 0018:ffffce09019efe00 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff8b5a56f819c0 RCX: 0000000000000017 RDX: 0000000000011696 RSI: 0000000000000003 RDI: ffffffffbc953b70 RBP: ffff8b5a4a314000 R08: 0000000000000001 R09: ffffffffc07d488a R10: fffffa6fc4787d80 R11: ffff8b5a40044000 R12: 0000000000000004 R13: 0000000000000003 R14: ffffce09003f1434 R15: ffff8b5a410c4640 FS: 0000000000000000(0000) GS:ffff8b5afe954000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ff463d65000 CR3: 000000012b476004 CR4: 0000000000772ef0 PKRU: 55555554 Call Trace:
<TASK> qxl_release_free+0xee/0xf0 [qxl
d93e9381353e619799d56790f5f8dda6cce491f6]
qxl_garbage_collect+0xd1/0x1b0 [qxl
d93e9381353e619799d56790f5f8dda6cce491f6]
process_one_work+0x19e/0x3a0 ...
The function does now:
void qxl_release_free(struct qxl_device *qdev, struct qxl_release *release) { ... if (dma_fence_was_initialized(&release->base)) { WARN_ON(list_empty(&release->bos));
qxl_release_free_list(release);
dma_fence_signal(&release->base);
dma_fence_put(&release->base);
refcount_dec_and_test() in kref_put() in this ^^ crashes. Apparently, the reference count was not increased anywhere. QXL does not call dma_fence_get() -- who is supposed to increase the refcount. And why it did not fail before?
} else {
qxl_release_free_list(release);
kfree(release);
}
atomic_dec(&qdev->release_count);
}
thanks,