Re: [PATCH v3 31/57] scsi: ipr: Prepare for enabling lock context analysis

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 concurrently while the underlying memory is freed.
--

--- Patch [31]: [PATCH v3 31/57] scsi: ipr: Prepare for enabling lock context 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 = 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 cancelled
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?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=31