[PATCH] scsi: mpi3mr: check the SMP passthrough reply status
Ilya Khomyakov <[email protected]>
| Newsgroups | org.kernel.vger.linux-scsi |
|---|---|
| Message-ID | <[email protected]> |
Hello. mpi3mr_transport_smp_handler() obtains the controller reply status from mpi3mr_post_transport_req() but never examines it. The status is written to the debug log and then discarded, after which the reply is copied to the caller and bsg_job_done() is called with a success code. mpi3mr_post_transport_req() returns zero once the request completes, regardless of the reported ioc_status. Its return value only reports a command already in use, a failure to submit the request, or a timeout. The existing "if (rc)" test therefore never observes a request that the controller refused. mpi3mr_report_manufacture() issues its SMP request through the same helper and tests ioc_status before using the reply. mpi3mr_transport_smp_handler() is the only caller that passes reply data to user space, and it performs no such test. As a result a refused SMP request reaches user space as a successful transfer of zero bytes. The BSG completion path derives the sg_io_v4 status fields from job->result, so with a result of zero device_status, transport_status and driver_status all read as zero and SG_INFO_CHECK is not set. The only remaining indication is din_resid, which stays equal to din_xfer_len. A caller that does not compare the two accepts the empty buffer as an SMP response frame and parses it. This was observed on a 9600-16e (SAS4116) running firmware 8.17.1.0 with personality 0 and profile id 3, that is PerfIT SAS Only mode, with an expander attached through an x8 wide port at 22.5 Gb/s. In that configuration the controller refuses SMP passthrough requests submitted through the BSG interface. Over roughly one hour the driver logged 109 such requests, every one of them as mpi3mr0: sending SMP request mpi3mr0: SMP request completed with ioc_status(0x0001) mpi3mr0: SMP request - reply data transfer size(0) ioc_status 0x0001 is MPI3_IOCSTATUS_INVALID_FUNCTION. All 109 were reported to the caller as successful transfers. Test the reply status and return -EIO when the controller did not complete the request successfully. The new exit uses the existing unmap_in label, so the DMA buffers are released exactly as before, and reslen is still zero at that point, so the length reported to the block layer stays consistent with the error code. On an error path the BSG completion path replaces job->reply_len with sizeof(u32), so the MPI reply structure that today carries ioc_status to user space is no longer copied out. That structure is driver specific and is not examined by smp_utils; returning a proper error code is the more useful result. The change was tested with a mpi3mr 8.17.1.0.0 build on the adapter described above. Before the change every refused request reached the "reply data transfer size" trace and completed successfully. After the change 21 refused requests over a four minute window all returned early: each was logged with ioc_status(0x0001) and none reached that trace, which is the expected signature of the new exit since it precedes the trace. During the same window mpi3mr_report_manufacture() issued one SMP request that the controller completed with ioc_status(0x0090), MPI3_IOCSTATUS_SAS_SMP_REQUEST_FAILED; that path already tests the status and was unaffected. This does not modify discovery, I/O or link management behavior. It changes only the result reported for an SMP passthrough request that the controller did not complete successfully. Signed-off-by: Ilya Khomyakov <[email protected]> --- drivers/scsi/mpi3mr/mpi3mr_transport.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c @@ -3320,6 +3320,11 @@ mpi3mr_transport_smp_handler(struct bsg_job *job, struct Scsi_Host *shost, dprint_transport_info(mrioc, "SMP request completed with ioc_status(0x%04x)\n", ioc_status); + if (ioc_status != MPI3_IOCSTATUS_SUCCESS) { + rc = -EIO; + goto unmap_in; + } + dprint_transport_info(mrioc, "SMP request - reply data transfer size(%d)\n", le16_to_cpu(mpi_reply.response_data_length));