[PATCH v3 0/2] scsi: elx: efct: fix two leaks on failure paths

[Ali Ahmet Memis]

Two of the five patches in v2 fix real leaks. The other three cleaned up
error paths that cannot be reached from their callers, so they are
dropped here. Sashiko's review of v2 3/5 pointed at the first of them,
and checking that properly showed the same problem in the other two.
Sorry for the churn.

  v2 1/5, HW state checked after the HIO is taken. hw->state is only
  non-ACTIVE during efct_hw_teardown(), during efct_hw_reset(), and
  before init. efct_hw_reset() has one caller, efct_fw_reset(), which
  follows it with efct_device_detach() and efct_device_attach(), and
  hw->io[] is torn down and rebuilt by that pair. So an entry stranded
  on hw->io_inuse does not outlive the adapter incarnation, and the
  "the pool runs dry and no further ELS or CT traffic can be sent"
  argument in that commit message does not hold.

  v2 2/5, HIO not returned when the WQE cannot be built. None of the
  builders reachable there can fail from these callers.
  sli_els_request64_wqe() and sli_xmit_els_rsp64_wqe() have no error
  return at all. sli_gen_request64_wqe() and sli_xmit_sequence64_wqe()
  fail only on !sgl || !sgl->virt, but efct_els_hw_srrs_send() has
  already dereferenced hio->sgl->virt before it reaches them.
  sli_xmit_bls_rsp64_wqe() fails on an unknown BLS type, which
  efct_hw_bls_send() itself sets to ACC or RJT, and on rpi_registered
  together with an s_id, which none of its three callers passes:
  efct_scsi.c sets s_id to U32_MAX and efc_send_bls_acc() leaves
  rpi_registered false.

  v2 3/5, request tag not returned when the send frame WQE fails.
  sli_send_frame_wqe() has a single "return 0", as Sashiko noted.

What is left is one leak that a failing dma_alloc_coherent() produces,
and one that I could measure. For the second, binding the driver to a
PCI device that is not an SLI-4 adapter makes sli_setup() fail after the
mempools have been created, and efct_pci_probe() then frees the struct
efct that held the only pointers to them. Repeating that probe 61 times
under CONFIG_DEBUG_KMEMLEAK:

  before   1566 unreferenced objects, every one from efct_hw_setup()
  after    none, and the probe still fails the same way

The first patch needs a real Emulex SLI-4 adapter and an allocation
failure, so it is reasoned from the code rather than exercised. Each
patch builds on its own.

Still not addressed here, and not claimed by these patches:
efct_xport_initialize() jumps past the hw_init_out label when
efct_hw_init() fails, so efct_hw_teardown() never runs and everything
efct_hw_init() managed to set up before failing stays behind, and
efcport_init() returns without destroying node_pool and node_dma_pool
when els_io_pool fails. Those cross two modules and I would rather send
them separately once this is settled.

v3: dropped v2 1/5, 2/5 and 3/5 for the reasons above; named the second
    caller of efct_hw_setup() and said what efct_hw_rx_buffer_free()
    actually frees.
v2: the mailbox pool patch claimed it cleared hw_setup_called on the way
    out while one of its three error paths still returned directly;
    fixed with a shared not_setup label.

Ali Ahmet Memis (2):
  scsi: elx: efct: free the RQ buffers already allocated when one fails
  scsi: elx: efct: destroy the mailbox pools when setup fails

 drivers/scsi/elx/efct/efct_hw.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)


base-commit: 0d839570765118029aa8bf4a95444c6a11aacf85
--
2.55.0