Re: [PATCH v3 31/57] scsi: ipr: Prepare for enabling lock context analysis
[email protected] Thu, 30 Jul 2026 23:15:13 +0000
| Newsgroups | org.kernel.vger.linux-scsi |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] Command timers are cancelled asynchronously during adapter removal= , which can lead to a use-after-free if the timer callback executes concurr= ently while the underlying memory is freed. -- --- Patch [31]: [PATCH v3 31/57] scsi: ipr: Prepare for enabling lock conte= xt analysis --- commit 2f00fc44ee9f9f3037991b9e96f4f40f0505e35f Author: Bart Van Assche <[email protected]> scsi: ipr: Prepare for enabling lock context analysis Document locking requirements with __must_hold(). > diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c > index d207e5e81afe0..51092834876cb 100644 > --- a/drivers/scsi/ipr.c > +++ b/drivers/scsi/ipr.c > @@ -1016,6 +1016,7 @@ static void ipr_init_ioadl(struct ipr_cmnd *ipr_cmd= , dma_addr_t dma_addr, > static void ipr_send_blocking_cmd(struct ipr_cmnd *ipr_cmd, > void (*timeout_func) (struct timer_list *), > u32 timeout) > + __must_hold(ipr_cmd->ioa_cfg->host->host_lock) > { > struct ipr_ioa_cfg *ioa_cfg =3D ipr_cmd->ioa_cfg; [Severity: High] This is a pre-existing issue, but can the asynchronous timer cancellation during adapter removal lead to a use-after-free? In drivers/scsi/ipr.c:ipr_fail_all_ops(), pending command timers are cancel= led using timer_delete(): drivers/scsi/ipr.c:ipr_fail_all_ops() { ... timer_delete(&ipr_cmd->timer); ... } Since this is non-synchronous, if a timer callback like ipr_timeout() is already running on another CPU, it will spin waiting for the lock: ioa_cfg->host->host_lock Meanwhile, the removal path in __ipr_remove() initiates the bringdown and eventually calls ipr_free_all_resources(). This frees the command blocks via dma_pool_free() in ipr_free_cmd_blks(). When the delayed timer callback finally acquires the lock, could it dereference the freed ipr_cmd and ioa_cfg structures? [ ... ] --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1785451174.gi= [email protected]?part=3D31