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

[Ilya Khomyakov <khomyakovilya@gmail.com>]

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. A user space daemon
on that system issued SMP passthrough requests that the controller refused.
Over roughly one hour the driver logged 109 of them, every one 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.

The driver's own SMP requests to the same expander complete normally on the
same system, for example

  mpi3mr0: report manufacturer SMP request completed with ioc_status(0x0000)
  mpi3mr0: report manufacturer - reply data transfer size(60)

so a status other than success here describes an individual request that
the controller declined, not a controller that lacks the function. Besides
MPI3_IOCSTATUS_INVALID_FUNCTION the firmware can report BUSY,
INSUFFICIENT_RESOURCES, INVALID_STATE, FAILURE, SAS_SMP_REQUEST_FAILED and
SAS_SMP_DATA_OVERRUN on this path, and all of them are currently returned
to user space as a successful transfer.

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.

Move the reply copy above the new test. When a job completes with a
negative result the BSG completion path shortens job->reply_len to
sizeof(u32) and returns those four bytes to the caller. job->reply points
at a buffer that is allocated once per tag in bsg_init_rq() and is not
cleared between requests, so leaving it untouched on the new exit would
return four bytes left over from whatever command last used that tag.
mpi_reply is zeroed before the request is submitted, so copying it
unconditionally makes those bytes describe this request. The error paths
that precede the request submission have the same property, but they are
not reached by a completed request and are left alone here.

The change was tested with a patched out-of-tree build of the same driver
on the adapter described above. Before the change all 109 refused requests
in a 64 minute window reached the "reply data transfer size" trace and
completed successfully. After the change all 141 refused requests in a 73
minute window 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. The daemon that issued the requests behaved
the same before and after the change. No warnings were reported by the
driver or the block layer during either run.

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 <khomyakovilya@gmail.com>

---
 drivers/scsi/mpi3mr/mpi3mr_transport.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

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,12 +3320,18 @@
 	dprint_transport_info(mrioc,
 	    "SMP request completed with ioc_status(0x%04x)\n", ioc_status);
 
+	memcpy(job->reply, &mpi_reply, reply_sz);
+	job->reply_len = reply_sz;
+
+	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));
 
-	memcpy(job->reply, &mpi_reply, reply_sz);
-	job->reply_len = reply_sz;
 	reslen = le16_to_cpu(mpi_reply.response_data_length);
 
 	if (addr_in)