[PATCH v6 0/5] scsi: mvumi: bound the values the firmware supplies

[Yehyeong Lee <yhlee@isslab.korea.ac.kr>]

This supersedes

  [PATCH v5 0/6] scsi: mvumi: bound the values the firmware supplies
  <20261014151914.3632701-1-yhlee@isslab.korea.ac.kr>

which can be dropped, and with it the two threads v5 already replaced:

  [PATCH v2 0/2] scsi: mvumi: bound two firmware-supplied lengths
  <20261013173145.3412863-1-yhlee@isslab.korea.ac.kr>

  [PATCH v4] scsi: mvumi: reject handshake sizes that overflow the list
  allocations
  <20261014134104.3528958-1-yhlee@isslab.korea.ac.kr>

Five patches instead of six: the widening of the mvumi_flush_cache() loop
counter is gone, replaced by a bound on the device count in the handshake
patch, and I owe a correction on v5 4/6 (below).

Everything the firmware writes reaches the driver either through the
handshake page, through the event buffer, or through a response frame in
the outbound list.  The patches follow that split:

 1  event buffer: param_count and sense_data_length index fixed arrays in
    mvumi_show_event().
 2  handshake page: frame_length drives the checksum walk, whose counter is
    an unsigned char, so 256 or more never terminates.
 3  event buffer: the hotplug bitmap size drives two find_next_zero_bit()
    scans with nothing holding it inside what the firmware was asked for.
 4  handshake page: the entry sizes and max_io size three allocations, the
    list depth drives the ring index, the device count has to fit the byte
    mvumi_build_frame() gives it, and a reset reports all of them again
    without reallocating anything.
 5  response frame: the tag indexes tag_cmd[], and the value that is
    checked and the value that is used are separate fetches of
    firmware-writable memory.

I have no mvumi hardware, so none of this is tested on a controller.  Each
patch's reasoning is from the source, and I modelled the arithmetic, the
loop bounds and the index ranges of every case in userspace, including the
values that should still be accepted.

A correction on v5 4/6: its commit message said it rejected an entry size
too small to hold the frames the driver puts in the entry.  The code did
not -- I lost that hunk before posting and did not catch it.  Patch 4 here
carries both the check and that wording.

v6:
- 4/6 "widen the flush cache loop counter" is dropped.  Patch 4 instead
  rejects a device count above 255, which is what mvumi_build_frame() can
  address: it packs the target id into the low byte of device_id and the
  LUN into the high byte, so a target of 256 or more aliases another
  device's LUN.  The same bound is what keeps the unsigned char counter in
  mvumi_flush_cache() terminating, so that loop needs no change.  This
  replaces what I wrote in reply to the v4 review, that rejecting such a
  count would turn away a working configuration -- it would not, the frame
  layout cannot address those targets at all.
- patch 4 regains the entry-size minimum described above.
- patch 3 bounds the bitmap by sizeof(struct mvumi_event_req), the length
  the driver puts in data_transfer_length, instead of by the 512-byte
  buffer.  dma_alloc_coherent() returns zeroed memory and a zero bit means
  a device is present, so the bytes past what the firmware wrote would be
  read as devices that do not exist.
- patch 5 uses READ_ONCE() for the tag and request_id it takes out of the
  outbound list, and patch 4 does the same for the three shift counts it
  takes out of the handshake page, matching patches 2 and 3.  Checking one
  fetch of a shift count and shifting by another is how a device gets past
  the bound those checks exist for.
- patches 1 and 2 are unchanged.

Yehyeong Lee (5):
  scsi: mvumi: bound the per-event parameter and sense dumps
  scsi: mvumi: reject an out-of-range handshake frame length
  scsi: mvumi: bound the hotplug event bitmap
  scsi: mvumi: reject handshake sizes that overflow the list allocations
  scsi: mvumi: validate the response frame tag before using it

 drivers/scsi/mvumi.c | 187 ++++++++++++++++++++++++++++++++++++-------
 drivers/scsi/mvumi.h |   9 +++
 2 files changed, 168 insertions(+), 28 deletions(-)


base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.43.0