[PATCH] scsi: target: sbp: Protect agent->orb_pointer and agent->doorbell with the lock

[PATCH] scsi: target: sbp: Protect agent->orb_pointer and agent->doorbell with the lock

[Ginger Li]

tgt_agent_rw_orb_pointer() and tgt_agent_rw_doorbell() update
agent->orb_pointer and agent->doorbell after they have dropped agent->lock,
while the work item tgt_agent_fetch_work() reads both fields to decide which
ORB to process.

The work item already accesses the fields under the lock when it updates them
and tgt_agent_check_active() takes the lock as well, so the unlocked stores
and this unlocked read race with each other.  The work item can then use an
ORB pointer that has been replaced by a newer request, or miss the doorbell
that belongs to the current one.

Update and read the two fields while holding agent->lock.

Fixes: a511ce339780 ("sbp-target: Initial merge of firewire/ieee-1394 target mode support")
Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
---
 drivers/target/sbp/sbp_target.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -721,10 +721,9 @@ static int tgt_agent_rw_orb_pointer(struct fw_card *ca
 			return RCODE_CONFLICT_ERROR;
 		}
 		agent->state = AGENT_STATE_ACTIVE;
-		spin_unlock_bh(&agent->lock);
-
 		agent->orb_pointer = sbp2_pointer_to_addr(ptr);
 		agent->doorbell = false;
+		spin_unlock_bh(&agent->lock);
 
 		pr_debug("tgt_agent ORB_POINTER write: 0x%llx\n",
 				agent->orb_pointer);
@@ -757,10 +756,9 @@ static int tgt_agent_rw_doorbell(struct fw_card *card,
 			return RCODE_CONFLICT_ERROR;
 		}
 		agent->state = AGENT_STATE_ACTIVE;
+		agent->doorbell = true;
 		spin_unlock_bh(&agent->lock);
 
-		agent->doorbell = true;
-
 		pr_debug("tgt_agent DOORBELL\n");
 
 		queue_work(system_dfl_wq, &agent->work);
@@ -933,9 +931,14 @@ static void tgt_agent_fetch_work(struct work_struct *w
 	struct sbp_session *sess = agent->login->sess;
 	struct sbp_target_request *req;
 	int ret;
-	bool doorbell = agent->doorbell;
-	u64 next_orb = agent->orb_pointer;
+	bool doorbell;
+	u64 next_orb;
 
+	spin_lock_bh(&agent->lock);
+	doorbell = agent->doorbell;
+	next_orb = agent->orb_pointer;
+	spin_unlock_bh(&agent->lock);
+
 	while (next_orb && tgt_agent_check_active(agent)) {
 		req = sbp_mgt_get_req(sess, sess->card, next_orb);
 		if (IS_ERR(req)) {
-- 
2.43.0