* Re: [PATCH] accel/qaic: tolerate absent crashdump state during cleanup
2026-09-13 12:52 [PATCH] accel/qaic: tolerate absent crashdump state during cleanup Slavin Liu
@ 2026-09-13 13:12 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-13 13:12 UTC (permalink / raw)
To: Slavin Liu; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing synchronization for `ssr_crash->dump_info` leads to Use-After-Free when accessed concurrently.
- [High] LIFO teardown ordering of DRM managed resources causes a Use-After-Free of `ssr_crash` in the workqueue.
- [High] Unchecked dereference of `ssr_crash` in `dbg_xfer_done_rsp` allows a malicious device to trigger a NULL pointer dereference.
- [High] Missing bounds checking on device-provided table entry sizes causes an integer overflow and out-of-bounds write.
- [High] Transient `ERR_PTR` stored in the globally visible `ssr_crash->dump_info` causes an invalid pointer dereference.
- [High] The `ssr_resp` receive buffer is permanently leaked if crashdump collection is aborted midway.
- [High] Missing synchronization between driver removal and the background workqueue leads to a NULL pointer dereference.
--
commit a18c3aaed1362f868bb5aefbf5fef4faf6c47707
Author: Slavin Liu <bolin.liu@seu.edu.cn>
accel/qaic: tolerate absent crashdump state during cleanup
A negative response can be sent before crashdump state is assigned.
If queuing that response fails, cleanup receives NULL. Treat that as
no owned crashdump resources instead of dereferencing it.
> diff --git a/drivers/accel/qaic/qaic_ssr.c b/drivers/accel/qaic/qaic_ssr.c
> index 418f14936f9a0..3eba7b1846b20 100644
> --- a/drivers/accel/qaic/qaic_ssr.c
> +++ b/drivers/accel/qaic/qaic_ssr.c
> @@ -195,7 +195,11 @@ struct dump_file_meta {
>
> static void free_ssr_dump_info(struct ssr_crashdump *ssr_crash)
> {
> - struct ssr_dump_info *dump_info = ssr_crash->dump_info;
> + struct ssr_dump_info *dump_info;
> +
> + if (!ssr_crash)
> + return;
> + dump_info = ssr_crash->dump_info;
>
> ssr_crash->dump_info = NULL;
[Severity: High]
This isn't a bug introduced by this patch, but does this clear the pointer and
proceed to free the structure locklessly?
A concurrently executing MHI asynchronous callback qaic_ssr_mhi_ul_xfer_cb
reads the pointer without locks:
drivers/accel/qaic/qaic_ssr.c:qaic_ssr_mhi_ul_xfer_cb() {
...
dump_info = ssr_crash->dump_info;
if (dump_info) {
dump_info->read_buf_req_queued = false;
return;
}
...
}
Can this lead to a use-after-free write if the asynchronous callback races with
the worker thread cleanup?
> if (!dump_info)
> return;
[Severity: High]
This is a pre-existing issue, but does free_ssr_dump_info() permanently leak
the ssr_resp receive buffer if crashdump collection is aborted midway?
A single MHI receive buffer is allocated and temporarily stored in
dump_info->resp while crashdump collection occurs:
drivers/accel/qaic/qaic_ssr.c:ssr_worker() {
...
dump_info->resp = resp;
...
}
When the structure is freed, the pointer is lost:
drivers/accel/qaic/qaic_ssr.c:free_ssr_dump_info() {
...
kfree(dump_info);
}
Does this drop the pointer without requeuing or freeing it?
[Severity: High]
This isn't a bug introduced by this patch, but does the LIFO teardown ordering
of DRM managed resources cause a use-after-free of ssr_crash in the workqueue?
The workqueue is registered for DRM managed destruction first:
drivers/accel/qaic/qaic_drv.c:create_qdev() {
...
qdev->ssr_wq = qaicm_wq_init(drm, "qaic_ssr");
...
}
The struct is registered later, meaning it will be freed before the workqueue
is flushed and destroyed:
drivers/accel/qaic/qaic_ssr.c:qaic_ssr_init() {
...
ssr_crash = drmm_kzalloc(drm, SSR_MHI_BUF_SIZE, GFP_KERNEL);
...
}
During the teardown flush, the worker dereferences the already-freed ssr_crash
object:
drivers/accel/qaic/qaic_ssr.c:ssr_dump_worker() {
struct ssr_crashdump *ssr_crash = container_of(work,
struct ssr_crashdump,
work);
...
}
[Severity: High]
This is a pre-existing issue, but is there an unchecked dereference of
ssr_crash in dbg_xfer_done_rsp()?
If memory allocation fails during probe, qdev->ssr_mhi_buf is left as NULL.
If a malicious device sends an unsolicited response, it unconditionally
dereferences it:
drivers/accel/qaic/qaic_ssr.c:dbg_xfer_done_rsp() {
struct ssr_crashdump *ssr_crash = qdev->ssr_mhi_buf;
...
dump_info = ssr_crash->dump_info;
...
}
Could this allow a misbehaving or compromised endpoint device to trigger a
NULL pointer dereference?
[Severity: High]
This isn't a bug introduced by this patch, but does the missing bounds
checking on device-provided table entry sizes cause an integer overflow?
In alloc_dump(), the device-controlled lengths are accumulated without overflow
checks:
drivers/accel/qaic/qaic_ssr.c:alloc_dump() {
...
while (tbl_sz_lp < dump_info->tbl_len) {
...
dump_size += tbl_ent->len;
...
}
...
}
This can cause an undersized allocation. Later, an out-of-bounds copy occurs
using the original lengths:
drivers/accel/qaic/qaic_ssr.c:ssr_copy_dump() {
...
memcpy(dump_info->dump_addr + dump_info->dump_off, data, len);
...
}
Can a compromised endpoint device exploit this to cause a heap buffer overflow?
[Severity: High]
This is a pre-existing issue, but does assigning an error pointer to
ssr_crash->dump_info cause an invalid pointer dereference?
If alloc_dump_info() fails, it temporarily exposes an ERR_PTR to a shared
global state accessible by concurrent interrupt handlers:
drivers/accel/qaic/qaic_ssr.c:dbg_xfer_info_rsp() {
...
ssr_crash->dump_info = alloc_dump_info(qdev, debug_info);
if (IS_ERR(ssr_crash->dump_info)) {
...
}
...
}
If qaic_ssr_mhi_ul_xfer_cb() executes concurrently, it checks if (dump_info),
which evaluates to true for an ERR_PTR, and attempts to dereference it. Can
this lead to a kernel panic?
[Severity: High]
This isn't a bug introduced by this patch, but is there missing synchronization
between driver removal and the background workqueue?
When the driver is removed, it zeroes out the MHI device pointer while
background workqueue tasks that depend on it may still be executing:
drivers/accel/qaic/qaic_ssr.c:qaic_ssr_mhi_remove() {
...
mhi_unprepare_from_transfer(qdev->ssr_ch);
qdev->ssr_ch = NULL;
}
A concurrently executing worker attempts to queue a buffer using the NULL
pointer:
drivers/accel/qaic/qaic_ssr.c:dbg_xfer_info_rsp() {
...
ret2 = mhi_queue_buf(qdev->ssr_ch, DMA_TO_DEVICE, debug_rsp,
sizeof(*debug_rsp), MHI_EOT);
...
}
Can an administrator unbinding the driver while the device is actively sending
events trigger a NULL pointer dereference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913125201.109964-1-bolin.liu@seu.edu.cn?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread