Skip to content

RFC scsi: lpfc: Fix mailbox timeout ownership races

Artem Dinaburg linux-scsi lkml stable 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 RFC Latest3 patches 0 repliesNew

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3scsi: lpfc: Do not touch the SFP mailbox after a wait timeoutdrivers/scsi/lpfc/lpfc_els.c+8 -40 replies
2/3scsi: lpfc: Resolve synchronous mailbox wait ownership under hbalockdrivers/scsi/lpfc/lpfc_sli.c+13 -160 replies
3/3scsi: lpfc: Add KUnit tests for mailbox wait ownershipdrivers/scsi/Kconfig drivers/scsi/lpfc/.kunitconfig drivers/scsi/lpfc/Makefile +1 more+389 -00 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [RFC PATCH 0/3] scsi: lpfc: Fix mailbox timeout ownership races

Hi Justin and Paul,

While preparing a 6.6.y backport of
commit ede596b1434b ("scsi: lpfc: Handle mailbox timeouts in lpfc_get_sfp_info"), I may have found two ownership problems in the synchronous mailbox timeout handoff. I am attaching patches just in case; I do not have the correct hardware (nor can I emulate it in QEMU) to validate the issue is actually reachable.

These patches were developed with AI assistance, and each carries the required attribution trailer.

Patch 1 fixes lpfc_get_sfp_info_wait(). After MBX_TIMEOUT it no longer reads the mailbox, which the late completion may already have freed, and it cleans up after any other failed issue instead of leaking the mailbox or reporting a zeroed A2 page as success. It is correct on its own with the current wait/wake code.

Patch 2 fixes the generic wait/wake handoff. A completion that loaded the wake callback before the waiter timed out finds no waiter and leaks the mailbox. Because the wake flag is set before hbalock is taken, a waiter that times out in that window can also return success and free the mailbox before the callback reads it. The wake callback now decides ownership under hbalock and runs the default completion itself when the waiter is gone.

Patch 3 adds a hardware-independent KUnit suite for the wait/wake paths and for lpfc_get_sfp_info_wait() itself. The issue failure cases fail without patch 1 and the stale-callback cases fail without patch 2. The tests check mailbox ownership only, not an SFP transaction or a timeout on an adapter.

I built lpfc from x86_64 allmodconfig with CONFIG_SCSI_LPFC=m and CONFIG_WERROR=y after each patch, and ran the KUnit suite in x86_64 QEMU with and without KASAN. KASAN only covers the paths the suite runs. I do not have LPFC hardware, so I have not exercised the SFP transaction on an adapter.

Patch 2 changes the completion path for every lpfc_sli_issue_mbox_wait() caller. I am not sure if this is the correct approach or if there should be a different fix.

Thanks,
Artem Dinaburg

Artem Dinaburg (3):
scsi: lpfc: Do not touch the SFP mailbox after a wait timeout
scsi: lpfc: Resolve synchronous mailbox wait ownership under hbalock
scsi: lpfc: Add KUnit tests for mailbox wait ownership

 drivers/scsi/Kconfig                 |  16 ++
 drivers/scsi/lpfc/.kunitconfig       |   9 +
 drivers/scsi/lpfc/Makefile           |   2 +
 drivers/scsi/lpfc/lpfc_els.c         |  12 +-
 drivers/scsi/lpfc/lpfc_sli.c         |  29 +--
 drivers/scsi/lpfc/tests/mbox_kunit.c | 362 +++++++++++++++++++++++++++
 6 files changed, 410 insertions(+), 20 deletions(-)
 create mode 100644 drivers/scsi/lpfc/.kunitconfig
 create mode 100644 drivers/scsi/lpfc/tests/mbox_kunit.c

base-commit: 5e0f8396d4805a3e7f753fa58c8c55f1f3cc2160

Signature · 1 line
2.43.0