Re: [PATCH 2/2] scsi: leapraid: serialize firmware log mmap with teardown

[Hao Dongdong]

在 2026/8/11 19:20, Linmao Li 写道:
> leapraid_fw_log_exit() waits for mmap_refcnt to reach zero before it
> frees the firmware log buffer.  leapraid_fw_mmap() checks
> host_removing, but it does not increment mmap_refcnt until after
> dma_mmap_coherent() succeeds and the VMA open callback runs.
> 
> Removal can set host_removing and observe a zero mmap_refcnt between
> the check and the VMA open.  It can then free the coherent buffer while
> the mmap path is still establishing a userspace mapping of it.
> 
> Claim a temporary mmap reference while looking up the adapter under
> leapraid_adapter_lock.  Removal deletes the adapter from the same
> locked list after setting host_removing, so a mapping is either
> rejected or included in the count that removal waits for.  Drop the
> temporary reference on the common exit path, after a successful VMA
> open has acquired the reference covering the VMA lifetime.
> 
> Fixes: 5597088c9e79 ("scsi: leapraid: Add new SCSI driver")
> Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> ---
>  drivers/scsi/leapraid/leapraid_app.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/leapraid/leapraid_app.c b/drivers/scsi/leapraid/leapraid_app.c
> index 841027ce2501c..c124f9eaea6ea 100644
> --- a/drivers/scsi/leapraid/leapraid_app.c
> +++ b/drivers/scsi/leapraid/leapraid_app.c
> @@ -171,7 +171,8 @@ static int leapraid_ctl_validate_sge_offset(struct leapraid_adapter *adapter,
>  	return 0;
>  }
>  
> -static struct leapraid_adapter *leapraid_ctl_lookup_adapter(int adapter_id)
> +static struct leapraid_adapter *
> +leapraid_ctl_lookup_adapter(int adapter_id, bool track_mmap)

Hi Linmao,

Thank you for identifying and fixing this race. Your analysis and fix
are correct.

I have only one minor comment: why the line break here? This declaration
fits within the line-length limit, so please keep the return type and
function name on the same line to follow the usual kernel coding style.

With this minor formatting issue addressed, I will be happy to accept
the patch. Thanks again!

Best regards,
Dongdong

>  {
>  	struct leapraid_adapter *adapter;
>  	struct Scsi_Host *shost;
> @@ -184,6 +185,8 @@ static struct leapraid_adapter *leapraid_ctl_lookup_adapter(int adapter_id)
>  			shost = adapter->shost;
>  			if (!shost || !scsi_host_get(shost))
>  				break;
> +			if (track_mmap)
> +				atomic_inc(&adapter->fw_log_desc.mmap_refcnt);
>  			spin_unlock(&leapraid_adapter_lock);
>  			return adapter;
>  		}
> @@ -589,7 +592,7 @@ static int leapraid_ctl_ioctl_main(struct file *file, unsigned int cmd,
>  		return -EFAULT;
>  	}
>  
> -	adapter = leapraid_ctl_lookup_adapter(ioctl_header.adapter_id);
> +	adapter = leapraid_ctl_lookup_adapter(ioctl_header.adapter_id, false);
>  	if (!adapter)
>  		return -EFAULT;
>  
> @@ -728,7 +731,7 @@ static int leapraid_fw_mmap(struct file *filp, struct vm_area_struct *vma)
>  
>  	length = vma->vm_end - vma->vm_start;
>  
> -	adapter = leapraid_ctl_lookup_adapter(adapter_id);
> +	adapter = leapraid_ctl_lookup_adapter(adapter_id, true);
>  	if (!adapter) {
>  		pr_err("%s: No adapter found!\n", __func__);
>  		return -EINVAL;
> @@ -771,6 +774,9 @@ static int leapraid_fw_mmap(struct file *filp, struct vm_area_struct *vma)
>  
>  	rc = 0;
>  out_put:
> +	if (adapter &&
> +	    atomic_dec_and_test(&adapter->fw_log_desc.mmap_refcnt))
> +		wake_up(&adapter->fw_log_desc.mmap_waitq);
>  	leapraid_ctl_put_adapter(adapter);
>  	return rc;
>  }