Re: [PATCH v3 2/2] scsi: leapraid: serialize firmware log mmap with teardown
| 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