[PATCH v5 0/3] scsi: bfa: bound the unsolicited frame receive path

[Yehyeong Lee]

Three fixes in the bfa unsolicited frame receive path. Together they bound
the length the path trusts, at both ends, from where it enters to where it
is used.

 1  uf_recv() takes the received length straight from the firmware's
    completion message and only warns if it is below sizeof(struct
    fchs_s). It is a 16-bit field and nothing caps it at the buffer that
    was posted, so every consumer below can be handed a length larger
    than the frame. The worst of them is bfa_fcs_lport_echo(), which uses
    it as a memcpy size into the fcxp request payload buffer, so that one
    is an out-of-bounds write and not just a read. Cap the length at the
    buffer that was posted.
 2  bfa_fcs_uf_recv() does not check the frame against the headers it has
    to hold. Below the FC header, bfa_fcs_lport_echo() computes a negative
    payload length and hands it to bfa_fcxp_send(), whose reqlen is a u32,
    so a six-byte frame asks the firmware to send about four gigabytes
    from the request payload address. Below the virtual fabric tagging
    header it advertises, stepping over that header wraps the length.
    Drop both, the way the function already drops a frame whose virtual
    fabric id is unknown.
 3  bfa_fcs_lport_scn_process_rscn() is handed that length and ignores it,
    taking the number of RSCN events to walk from the peer's payload
    length field alone. Bound the walk by the received frame length, and
    reject a payload length below the RSCN header it is measured from.

Patches 1 and 2 are the source and patch 3 is one of the sinks; each
stands on its own, and the first two come first because they are what
makes the length the third uses meaningful. With both, every consumer
sees a length between the FC header and the posted buffer.

None of this is tested on hardware; I have no Brocade adapter. All three
were driven through a KUnit harness on a KASAN kernel that injects a frame
completion into uf_recv(), a frame into bfa_fcs_uf_recv(), and frames into
the RSCN and ECHO handlers directly. Without patch 1 a reported length of
2400 reaches bfa_fcs_lport_echo() and its memcpy runs 72 bytes past the
2304-byte request buffer. Without patch 2 a six-byte frame reaches the
same handler and leaves 4294967278 in the request length it hands the
firmware, and a six-byte tagged frame hands the FCS a length of 65534.
Without patch 3 an RSCN whose payload length exceeds the frame reads past
the frame buffer. With all three, a header-only frame of 24 bytes, a frame
of FC_MAX_PDUSZ and a legitimate RSCN, including one that fills the frame
exactly, are all unaffected.

v5: also drop frames shorter than the mandatory FC header in
    bfa_fcs_uf_recv(), closing a pre-existing lower-bound underflow that
    reaches bfa_fcs_lport_echo().
v4: added the cap on the frame length itself in uf_recv(); v3 bounded the
    RSCN walk and the VFT underflow but still trusted a firmware-reported
    length that bfa_fcs_lport_echo() uses to drive an out-of-bounds
    memcpy.
v3: added the VFT-header length underflow guard.
v2: bound the RSCN walk by the received frame length only (dropped a
    buffer-capacity term that did not account for the VFT header offset);
    reject payloads shorter than the RSCN header.

Yehyeong Lee (3):
  scsi: bfa: clamp the received unsolicited frame length to the buffer
  scsi: bfa: drop unsolicited frames shorter than the headers they must
    hold
  scsi: bfa: bound the RSCN event list by the received frame length

 drivers/scsi/bfa/bfa_fcs.c       | 15 ++++++++++++++-
 drivers/scsi/bfa/bfa_fcs_lport.c | 19 +++++++++++++++----
 drivers/scsi/bfa/bfa_svc.c       |  2 +-
 3 files changed, 30 insertions(+), 6 deletions(-)


base-commit: 2f2f7eeef8b0a1a9b9ca998231419873ccae8d4e
-- 
2.43.0