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

[email protected]
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] Unbounded wait in `leapraid_fw_log_exit()` allows userspace to block PCI device removal indefinitely.
--

--- Patch [2]: [PATCH v3 2/2] scsi: leapraid: serialize firmware log mmap with teardown ---
commit f76020cc63b98ce5d018410c191835d8ffc2261a
Author: Linmao Li <[email protected]>

scsi: leapraid: serialize firmware log mmap with teardown

This commit fixes a race condition during firmware log teardown by
claiming a temporary mmap reference during adapter lookup. It ensures
the driver does not free the coherent buffer while the mmap path is
still establishing a mapping.

> diff --git a/drivers/scsi/leapraid/leapraid_app.c b/drivers/scsi/leapraid/leapraid_app.c
> index 841027ce2501c..84eef50947aea 100644
> --- a/drivers/scsi/leapraid/leapraid_app.c
> +++ b/drivers/scsi/leapraid/leapraid_app.c

[ ... ]

> @@ -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);

[Severity: High]
This is a pre-existing issue, but since we are modifying the mmap teardown
synchronization here, I noticed a potential problem on the wait side of this
logic.

If a userspace process keeps the VMA open indefinitely, won't the unbind
process hang forever?

Looking at the teardown side in drivers/scsi/leapraid/leapraid_func.c:

leapraid_fw_log_exit() {
    wait_event(adapter->fw_log_desc.mmap_waitq,
               !atomic_read(&adapter->fw_log_desc.mmap_refcnt));
}

Since this waits in uninterruptible sleep without a timeout, could a rogue
or stuck userspace application block PCI device removal or module unloading
permanently? 

It seems like the kernel shouldn't rely on cooperative userspace behavior 
during hardware unbind, perhaps this needs an unmap_mapping_range() or 
similar asynchronous shootdown mechanism to ensure the wait completes?

>  	leapraid_ctl_put_adapter(adapter);
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.