Re: [PATCH V2] scsi: mpi3mr: check the SMP passthrough reply status

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check on user-provided payload lengths causes integer underflow, leading to massive out-of-bounds DMA.
- [High] mpi3mr_transport_phy_reset ignores the controller's reply status (ioc_status) when resetting HBA phys, falsely reporting success on failure.
- [Medium] Unbounded firmware response length triggers a WARN_ON in the generic BSG layer.
- [Low] Early error paths in BSG handlers leak 4 bytes of stale kernel heap memory to user space via the uninitialized `job->reply` buffer.
--

--- Patch [1]: [PATCH V2] scsi: mpi3mr: check the SMP passthrough reply status ---
Note: The format of this report has been altered to avoid quoting the
original patch code directly, due to recitation policy restrictions. The
findings are provided as a free-form summary below.

Commit: 8f30f09fa87333575dd13664d3ca77572ba9a47e
Author: Ilya Khomyakov <khomyakovilya@gmail.com>
Subject: scsi: mpi3mr: check the SMP passthrough reply status
Summary: The patch modifies mpi3mr_transport_smp_handler to check the
controller's reply status from mpi3mr_post_transport_req. It ensures refused
SMP requests correctly return an error code instead of falsely reporting
success.

[Severity: Low]
This isn't a bug introduced by this patch, but looking at the early error
paths in mpi3mr_transport_smp_handler (for example, when checking
mrioc->reset_in_progress), the function exits to the out label without
populating job->reply. Does this leak 4 bytes of stale kernel memory to
user space? Since job->reply is reused across requests without zeroing,
the bsg core completing the job will truncate the reply length to 4 bytes
and copy uninitialized or stale data to the user's response buffer.

[Severity: Medium]
This is a pre-existing issue, but further down in
mpi3mr_transport_smp_handler, the code assigns reslen directly from the
controller's mpi_reply.response_data_length. It then passes reslen to
bsg_job_done without bounding it against the user-allocated
job->reply_payload.payload_len. Could this trigger a WARN_ON in the
generic BSG layer? If the firmware returns a response length greater than
the allocated buffer, the BSG core's bsg_transport_sg_io_fn triggers a
WARN_ON for the mismatched lengths, which could act as a denial of service
vector if panic_on_warn is configured.

[Severity: High]
This is also a pre-existing issue, but I noticed a potential integer
underflow when setting up the scatter-gather elements in
mpi3mr_transport_smp_handler. The code subtracts 4 from dma_len_out and
dma_len_in when calling mpi3mr_add_sg_single. What happens if a user
passes a payload length of 1 to 3 bytes? Since there is no bounds check
ensuring the payload is at least 4 bytes, wouldn't this subtraction
underflow to a massive 32-bit unsigned integer and cause out-of-bounds DMA
operations?

[Severity: High]
This isn't a bug introduced by this patch, but I found a very similar
logic error to the one you just fixed, located nearby in
mpi3mr_transport_phy_reset. When resetting an HBA phy, it calls
mpi3mr_post_transport_req and logs the ioc_status, but then completely
ignores the status value and falls through to return success. Can this
disrupt SAS link recovery? If the controller actively refuses the reset,
falsely reporting success might cause the transport layer to incorrectly
assume the phy is operational.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817143841.1856-1-khomyakovilya@gmail.com?part=1