Re: [PATCH v2 5/5] scsi: elx: efct: destroy the mailbox pools when setup fails

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The global list `efct_devices` is modified and accessed concurrently without any locking, causing list corruption and use-after-free races.
--

--- Patch [5]: [PATCH v2 5/5] scsi: elx: efct: destroy the mailbox pools when setup fails ---
commit 250c50655b4add3b10d0ee9bad5b35495210fdcb
Author: Ali Ahmet Memis <[email protected]>

scsi: elx: efct: destroy the mailbox pools when setup fails

This commit prevents memory leaks by destroying mailbox mempools in 
the error paths of efct_hw_setup() before returning.

> mempool_destroy() for these two runs only in efct_hw_teardown(), which is
> not reached here. efct_hw_setup() is called from
> efct_device_interrupts_required(), and when it fails efct_pci_probe()
> unwinds through efct_device_free(), freeing the struct efct that held the
> only pointers to the pools.

[Severity: High]
This is a pre-existing issue, but since the probe error path unwinds through 
efct_device_free(), does this code safely handle concurrent accesses to the 
global device list?

Looking at efct_device_alloc() in drivers/scsi/elx/efct/efct_driver.c, the 
device is added to the global efct_devices list without locking:

    list_add_tail(&efct->list_entry, &efct_devices);

When unwinding through efct_device_free(), it is removed and immediately freed:

    list_del(&efct->list_entry);
    kfree(efct);

Meanwhile, userspace configfs operations can trigger efct_find_wwpn() in 
drivers/scsi/elx/efct/efct_lio.c, which traverses this same list locklessly:

    list_for_each_entry(efct, &efct_devices, list_entry) {

Could concurrent probe/remove operations and configfs reads race here, 
causing list_for_each_entry() to hit corrupted list pointers or trigger 
a use-after-free on the released efct object?

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