With a recent change to our send path for FSF commands we introduced a
possible use-after-free of request-objects, that might further lead to
zfcp crafting bad requests, which the FCP channel correctly complains
about with an error (FSF_PROT_SEQ_NUMB_ERROR). This error is then handled
by an adapter-wide recovery.
The following sequence illustrates the possible use-after-free:
static int zfcp_fsf_req_send(struct zfcp_fsf_req *req)
{
struct zfcp_adapter *adapter = req->adapter;
struct zfcp_qdio *qdio = adapter->qdio;
...
zfcp_reqlist_add(adapter->req_list, req);
// ^^^^^^^^^^^^^^^^
// add request to our driver-internal hash-table for tracking
// (protected by separate lock req_list->lock)
...
if (zfcp_qdio_send(qdio, &req->qdio_req)) {
// ^^^^^^^^^^^^^^
// hand-off the request to FCP channel;
// the request can complete at any point now
...
}
/* Don't increase for unsolicited status */
if (!zfcp_fsf_req_is_status_read_buffer(req))
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// possible use-after-free
adapter->fsf_req_seq_no++;
// ^^^^^^^^^^^^^^^^
// because of the use-after-free we might
// miss this accounting, and as follow-up
// this results in the FCP channel error
// FSF_PROT_SEQ_NUMB_ERROR
adapter->req_no++;
static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req)
{
...
if (likely(req->status & ZFCP_STATUS_FSFREQ_CLEANUP))
zfcp_fsf_req_free(req);
// ^^^^^^^^^^^^^^^^^
// free memory for request-object
else
complete(&req->completion);
// ^^^^^^^^
// completion notification for code-paths that wait
// synchronous for the completion of the request; in
// those the memory is freed separately
}
The result of the use-after-free only affects the send path, and can not
lead to any data corruption. In case we miss the sequence-number
accounting, because the memory was already re-purposed, the next FSF
command will fail with said FCP channel error, and we will recover the
whole adapter. This causes no additional errors, but it slows down
traffic. There is a slight chance of the same thing happen again
recursively after the adapter recovery, but so far this has not been seen.
This was seen under z/VM, where the send path might run on a virtual CPU
that gets scheduled away by z/VM, while the return path might still run,
and so create the necessary timing. Running with KASAN can also slow down
the kernel sufficiently to run into this user-after-free, and then see the
report by KASAN.
To fix this, simply pull the test for the sequence-number accounting in
front of the hand-off to the FCP channel (this information doesn't change
during hand-off), but leave the sequence-number accounting itself where it
is.
To make future regressions of the same kind less likely, add comments to
all closely related code-paths.
Signed-off-by: Benjamin Block <bblock@linux.ibm.com> Fixes: f9eca0227600 ("scsi: zfcp: drop duplicate fsf_command from zfcp_fsf_req which is also in QTCB header") Cc: <stable@vger.kernel.org> #5.0+ Reviewed-by: Steffen Maier <maier@linux.ibm.com> Reviewed-by: Jens Remus <jremus@linux.ibm.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>