[ewg] [PATCH 2/2 for compat-rdma] ib_srp: Add post-3.5 upstream patches
Bart Van Assche
bvanassche at acm.org
Thu Oct 18 05:47:21 PDT 2012
The following three patches have been accepted upstream after kernel
3.5 was released:
- IB-srp-Fix-a-race-condition: avoid that late replies can trigger
a crash.
- IB-srp-Fix-use-after-free-in-srp_reset_req
- IB-srp-Avoid-having-aborted-requests-hang
Add these patches to OFED 3.5.
Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
patches/0026-IB-srp-Fix-a-race-condition.patch | 160 ++++++++++++++++++++
...B-srp-Fix-use-after-free-in-srp_reset_req.patch | 35 +++++
...IB-srp-Avoid-having-aborted-requests-hang.patch | 30 ++++
3 files changed, 225 insertions(+)
create mode 100644 patches/0026-IB-srp-Fix-a-race-condition.patch
create mode 100644 patches/0027-IB-srp-Fix-use-after-free-in-srp_reset_req.patch
create mode 100644 patches/0028-IB-srp-Avoid-having-aborted-requests-hang.patch
diff --git a/patches/0026-IB-srp-Fix-a-race-condition.patch b/patches/0026-IB-srp-Fix-a-race-condition.patch
new file mode 100644
index 0000000..969a3c1
--- /dev/null
+++ b/patches/0026-IB-srp-Fix-a-race-condition.patch
@@ -0,0 +1,160 @@
+From 220329916c72ee3d54ae7262b215a050f04a18fc Mon Sep 17 00:00:00 2001
+From: Bart Van Assche <bvanassche at acm.org>
+Date: Tue, 14 Aug 2012 13:18:53 +0000
+Subject: [PATCH] IB/srp: Fix a race condition
+
+Avoid a crash caused by the scmnd->scsi_done(scmnd) call in
+srp_process_rsp() being invoked with scsi_done == NULL. This can
+happen if a reply is received during or after a command abort.
+
+Reported-by: Joseph Glanville <joseph.glanville at orionvm.com.au>
+Reference: http://marc.info/?l=linux-rdma&m=134314367801595
+Cc: <stable at vger.kernel.org>
+Acked-by: David Dillow <dillowda at ornl.gov>
+Signed-off-by: Bart Van Assche <bvanassche at acm.org>
+Signed-off-by: Roland Dreier <roland at purestorage.com>
+---
+ drivers/infiniband/ulp/srp/ib_srp.c | 87 +++++++++++++++++++++++++----------
+ 1 file changed, 63 insertions(+), 24 deletions(-)
+
+diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
+index bcbf22e..1b5b0c7 100644
+--- a/drivers/infiniband/ulp/srp/ib_srp.c
++++ b/drivers/infiniband/ulp/srp/ib_srp.c
+@@ -586,24 +586,62 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
+ scmnd->sc_data_direction);
+ }
+
+-static void srp_remove_req(struct srp_target_port *target,
+- struct srp_request *req, s32 req_lim_delta)
++/**
++ * srp_claim_req - Take ownership of the scmnd associated with a request.
++ * @target: SRP target port.
++ * @req: SRP request.
++ * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take
++ * ownership of @req->scmnd if it equals @scmnd.
++ *
++ * Return value:
++ * Either NULL or a pointer to the SCSI command the caller became owner of.
++ */
++static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
++ struct srp_request *req,
++ struct scsi_cmnd *scmnd)
++{
++ unsigned long flags;
++
++ spin_lock_irqsave(&target->lock, flags);
++ if (!scmnd) {
++ scmnd = req->scmnd;
++ req->scmnd = NULL;
++ } else if (req->scmnd == scmnd) {
++ req->scmnd = NULL;
++ } else {
++ scmnd = NULL;
++ }
++ spin_unlock_irqrestore(&target->lock, flags);
++
++ return scmnd;
++}
++
++/**
++ * srp_free_req() - Unmap data and add request to the free request list.
++ */
++static void srp_free_req(struct srp_target_port *target,
++ struct srp_request *req, struct scsi_cmnd *scmnd,
++ s32 req_lim_delta)
+ {
+ unsigned long flags;
+
+- srp_unmap_data(req->scmnd, target, req);
++ srp_unmap_data(scmnd, target, req);
++
+ spin_lock_irqsave(&target->lock, flags);
+ target->req_lim += req_lim_delta;
+- req->scmnd = NULL;
+ list_add_tail(&req->list, &target->free_reqs);
+ spin_unlock_irqrestore(&target->lock, flags);
+ }
+
+ static void srp_reset_req(struct srp_target_port *target, struct srp_request *req)
+ {
+- req->scmnd->result = DID_RESET << 16;
+- req->scmnd->scsi_done(req->scmnd);
+- srp_remove_req(target, req, 0);
++ struct scsi_cmnd *scmnd = srp_claim_req(target, req, NULL);
++
++ if (scmnd) {
++ scmnd->result = DID_RESET << 16;
++ scmnd->scsi_done(scmnd);
++ srp_free_req(target, req, scmnd, 0);
++ }
+ }
+
+ static int srp_reconnect_target(struct srp_target_port *target)
+@@ -1073,11 +1111,18 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
+ complete(&target->tsk_mgmt_done);
+ } else {
+ req = &target->req_ring[rsp->tag];
+- scmnd = req->scmnd;
+- if (!scmnd)
++ scmnd = srp_claim_req(target, req, NULL);
++ if (!scmnd) {
+ shost_printk(KERN_ERR, target->scsi_host,
+ "Null scmnd for RSP w/tag %016llx\n",
+ (unsigned long long) rsp->tag);
++
++ spin_lock_irqsave(&target->lock, flags);
++ target->req_lim += be32_to_cpu(rsp->req_lim_delta);
++ spin_unlock_irqrestore(&target->lock, flags);
++
++ return;
++ }
+ scmnd->result = rsp->status;
+
+ if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {
+@@ -1092,7 +1137,9 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
+ else if (rsp->flags & (SRP_RSP_FLAG_DIOVER | SRP_RSP_FLAG_DIUNDER))
+ scsi_set_resid(scmnd, be32_to_cpu(rsp->data_in_res_cnt));
+
+- srp_remove_req(target, req, be32_to_cpu(rsp->req_lim_delta));
++ srp_free_req(target, req, scmnd,
++ be32_to_cpu(rsp->req_lim_delta));
++
+ scmnd->host_scribble = NULL;
+ scmnd->scsi_done(scmnd);
+ }
+@@ -1631,25 +1678,17 @@ static int srp_abort(struct scsi_cmnd *scmnd)
+ {
+ struct srp_target_port *target = host_to_target(scmnd->device->host);
+ struct srp_request *req = (struct srp_request *) scmnd->host_scribble;
+- int ret = SUCCESS;
+
+ shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
+
+- if (!req || target->qp_in_error)
++ if (!req || target->qp_in_error || !srp_claim_req(target, req, scmnd))
+ return FAILED;
+- if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
+- SRP_TSK_ABORT_TASK))
+- return FAILED;
+-
+- if (req->scmnd) {
+- if (!target->tsk_mgmt_status) {
+- srp_remove_req(target, req, 0);
+- scmnd->result = DID_ABORT << 16;
+- } else
+- ret = FAILED;
+- }
++ srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
++ SRP_TSK_ABORT_TASK);
++ srp_free_req(target, req, scmnd, 0);
++ scmnd->result = DID_ABORT << 16;
+
+- return ret;
++ return SUCCESS;
+ }
+
+ static int srp_reset_device(struct scsi_cmnd *scmnd)
+--
+1.7.10.4
+
diff --git a/patches/0027-IB-srp-Fix-use-after-free-in-srp_reset_req.patch b/patches/0027-IB-srp-Fix-use-after-free-in-srp_reset_req.patch
new file mode 100644
index 0000000..f9b3e24
--- /dev/null
+++ b/patches/0027-IB-srp-Fix-use-after-free-in-srp_reset_req.patch
@@ -0,0 +1,35 @@
+From 9b796d06d5d1b1e85ae2316a283ea11dd739ef96 Mon Sep 17 00:00:00 2001
+From: Bart Van Assche <bvanassche at acm.org>
+Date: Fri, 24 Aug 2012 10:27:54 +0000
+Subject: [PATCH] IB/srp: Fix use-after-free in srp_reset_req()
+
+srp_free_req() uses the scsi_cmnd structure contents to unmap
+buffers, so we must invoke srp_free_req() before we release
+ownership of that structure.
+
+Signed-off-by: Bart Van Assche <bvanassche at acm.org>
+Acked-by: David Dillow <dillowda at ornl.gov>
+Cc: <stable at vger.kernel.org>
+Signed-off-by: Roland Dreier <roland at purestorage.com>
+---
+ drivers/infiniband/ulp/srp/ib_srp.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
+index 1b5b0c7..ac66e6b 100644
+--- a/drivers/infiniband/ulp/srp/ib_srp.c
++++ b/drivers/infiniband/ulp/srp/ib_srp.c
+@@ -638,9 +638,9 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *re
+ struct scsi_cmnd *scmnd = srp_claim_req(target, req, NULL);
+
+ if (scmnd) {
++ srp_free_req(target, req, scmnd, 0);
+ scmnd->result = DID_RESET << 16;
+ scmnd->scsi_done(scmnd);
+- srp_free_req(target, req, scmnd, 0);
+ }
+ }
+
+--
+1.7.10.4
+
diff --git a/patches/0028-IB-srp-Avoid-having-aborted-requests-hang.patch b/patches/0028-IB-srp-Avoid-having-aborted-requests-hang.patch
new file mode 100644
index 0000000..586d5b9
--- /dev/null
+++ b/patches/0028-IB-srp-Avoid-having-aborted-requests-hang.patch
@@ -0,0 +1,30 @@
+From d8536670916a685df116b5c2cb256573fd25e4e3 Mon Sep 17 00:00:00 2001
+From: Bart Van Assche <bvanassche at acm.org>
+Date: Fri, 24 Aug 2012 10:29:11 +0000
+Subject: [PATCH] IB/srp: Avoid having aborted requests hang
+
+We need to call scsi_done() for commands after we abort them.
+
+Signed-off-by: Bart Van Assche <bvanassche at acm.org>
+Acked-by: David Dillow <dillowda at ornl.gov>
+Cc: <stable at vger.kernel.org>
+Signed-off-by: Roland Dreier <roland at purestorage.com>
+---
+ drivers/infiniband/ulp/srp/ib_srp.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
+index ac66e6b..922d845 100644
+--- a/drivers/infiniband/ulp/srp/ib_srp.c
++++ b/drivers/infiniband/ulp/srp/ib_srp.c
+@@ -1687,6 +1687,7 @@ static int srp_abort(struct scsi_cmnd *scmnd)
+ SRP_TSK_ABORT_TASK);
+ srp_free_req(target, req, scmnd, 0);
+ scmnd->result = DID_ABORT << 16;
++ scmnd->scsi_done(scmnd);
+
+ return SUCCESS;
+ }
+--
+1.7.10.4
+
--
1.7.10.4
More information about the ewg
mailing list