Skip to contentSkip to conversation

zfcp: fix use after free bug

PATCH New Swen Schillig 1 participant 1 message Nov 23, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 zfcp: fix use after free bug

From: Heiko Carstens <[email protected]>

zfcp_erp_strategy_check_fsfreq() checks if it is safe to access the fsf_req associated with the erp_action that gets passed. To test if it is safe it accesses the fsf_req in order to get its index into the hash list. This is broken since the fsf_req might be freed already and the read index has no meaning. It could lead to memory corruption. Fix this by introducing a new zfcp_reqlist_find_safe() method which just checks if addresses are equal. This is slower, but only gets called in case of error recovery.

---

 drivers/s390/scsi/zfcp_def.h |   14 ++++++++++++++
 drivers/s390/scsi/zfcp_erp.c |    3 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/s390/scsi/zfcp_def.h +14 -0
Index: SHIP_OCT2005/drivers/s390/scsi/zfcp_def.h
===================================================================
@@ -1210,6 +1210,20 @@ zfcp_reqlist_find(struct zfcp_adapter *a
 	return NULL;
 }
 
+static inline struct zfcp_fsf_req *
+zfcp_reqlist_find_safe(struct zfcp_adapter *adapter, struct zfcp_fsf_req *req)
+{
+	struct zfcp_fsf_req *request;
+	unsigned int idx;
+
+	for (idx = 0; idx < REQUEST_LIST_SIZE; idx++) {
+		list_for_each_entry(request, &adapter->req_list[idx], list)
+			if (request == req)
+				return request;
+	}
+	return NULL;
+}
+
 /*
  *  functions needed for reference/usage counting
  */
drivers/s390/scsi/zfcp_erp.c +2 -1
Index: SHIP_OCT2005/drivers/s390/scsi/zfcp_erp.c
===================================================================
@@ -837,7 +837,8 @@ zfcp_erp_strategy_check_fsfreq(struct zf
 	if (erp_action->fsf_req) {
 		/* take lock to ensure that request is not deleted meanwhile */
 		spin_lock(&adapter->req_list_lock);
-		if (zfcp_reqlist_find(adapter, erp_action->fsf_req->req_id)) {
+		if (zfcp_reqlist_find_safe(adapter, erp_action->fsf_req) &&
+		    erp_action->fsf_req->erp_action == erp_action) {
 			/* fsf_req still exists */
 			debug_text_event(adapter->erp_dbf, 3, "a_ca_req");
  			debug_event(adapter->erp_dbf, 3, &erp_action->fsf_req,