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

Ilya Khomyakov <[email protected]>
Newsgroups gmane.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));
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.