[PATCH v4 0/3] IB/isert: fix the device removal use-after-free

[Yehyeong Lee]

The RDMA_CM_EVENT_DEVICE_REMOVAL callback in ib_isert waits for the
connection it was handed to reach ISER_CONN_DOWN and then frees it, which
is a use-after-free against isert_connect_release(), and the wait itself
deadlocks against the handler_mutex that rdma_destroy_id() needs.  3/3
moves the teardown into an ib_client callback, as nvmet-rdma did in commit
f1d4ef7d8883.  1/3 is a separate problem that 3/3 adds a caller to, and
2/3 is an older deadlock that 3/3 would otherwise inherit.

The design is one owner per connection: publishing happens in the same
isert_conn_mutex section that tests isert_device->removing, and whoever
empties isert_conn->node under isert_np->mutex owns the release.

v3 -> v4

v3: https://lore.kernel.org/r/20261102061608.617186-1-yhlee@xxxxxxxxxxxxxxxxxx

Review of v3 raised six problems, all real; the first was a regression.

The refusal path in isert_connect_request() was that regression: v3 tested
removing after isert_rdma_accept() had run, so it destroyed an accepted QP
without draining it and rejected an id it had accepted.  v4 tests removing
before the accept and rejects there; if the test at the publish point
catches it, v4 drains the QP and returns an error so cma destroys the id.

Draining under isert_conn->mutex was the second, and not v3's doing:
mainline has done it since commit 6d1fba0c2cc7 ("iser-target: Rework
connection termination").  2/3 fixes it, and I split the drain fix out
with its own Fixes tag so it can reach stable without the rest.

The connection left in ISER_CONN_INIT was the third.  v3 marked it and
waited for a CM event that never comes; in v4 the removal walk owns it.

The trigger review named there, a DREQ before the RTU, needs the RTU lost
after the initiator sent it; a test-only drop left eight connections
unreleased on v3 with the removal never finishing, and none on v4.

The fourth and fifth were races on the np lists, both in 1/3.  The CM
handlers test isert_conn->node under isert_np->mutex and stand back when
it is empty, and connect_error returns zero there.

The sixth, a CONNECT_REQUEST after isert_remove_one() returns, is still
open: a test-only delay showed a new isert_device torn down cleanly by
cma's DEVICE_REMOVAL, but nothing orders that against device release.

One change is mine: the teardown now runs outside isert_conn_mutex, which
let a connection outlive its portal, so isert_np is reference counted.

With soft-RoCE, three v3 against three v4 runs per case: accept and INIT
went from a removal that never finished to one finished in seconds, drain
from six stuck drains to none, np list from 32 use after frees to none.
2/3 alone turned ten hung task warnings a run into none, three runs each.
Under KASAN one of twelve unpatched runs finished the removal and two
reported the use-after-free, 17 and 11 times; all twelve patched runs
finished and reported nothing.  Making rdma_accept() fail on every fourth
connection refused 48 connections over three runs with no KASAN report,
and every removal finished.  Login and I/O ran 18 times a side with no
difference and PROVE_LOCKING gave no splat in eight runs.

Yehyeong Lee (3):
  IB/isert: do not publish connections being released on a second list
  IB/isert: drain the QP without holding isert_conn->mutex
  IB/isert: tear connections down from an ib_client on device removal

 drivers/infiniband/ulp/isert/ib_isert.c | 310 ++++++++++++++++++------
 drivers/infiniband/ulp/isert/ib_isert.h |   9 +-
 2 files changed, 240 insertions(+), 79 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0