Skip to contentSkip to conversation

scsi: dpt_i2o: use after free in adpt_release()

PATCH Applied Dan Carpenter 3 participants 5 messages Mar 19, 2018 – Mar 21, 2018

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

PATCH 1/2 scsi: dpt_i2o: use after free in adpt_release()

The scsi_host_put() function frees "pHba" and then we dereference it on the next line when we do "scsi_host_put(pHba->host);".

Fixes 38e09e3bb056 ("scsi: dpt_i2o: stop using scsi_unregister")
drivers/scsi/dpt_i2o.c +1 -1
index 3c667b23a801..ac2f40d9963b 100644
@@ -306,8 +306,8 @@ static void adpt_release(adpt_hba *pHba)
 {
 	scsi_remove_host(pHba->host);
 //	adpt_i2o_quiesce_hba(pHba);
-	adpt_i2o_delete_hba(pHba);
 	scsi_host_put(pHba->host);
+	adpt_i2o_delete_hba(pHba);
 }
 
 
#1 lore raw

Dan,

Quoted 2 lines

The scsi_host_put() function frees "pHba" and then we dereference it on the next line when we do "scsi_host_put(pHba->host);".

Applied to 4.17/scsi-queue, thank you.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
#2 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Martin K. Petersen

Dan,

Quoted 2 lines

The scsi_host_put() function frees "pHba" and then we dereference it on the next line when we do "scsi_host_put(pHba->host);".

Applied to 4.17/scsi-queue, thank you.

This fix is broken! adpt_i2o_delete_hba references pHba->host as well.

Instead we need a local variable for the host. Fix below:

---
From 701440055539c0f72a3179d85a44bd59d45a7d4b Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Tue, 20 Mar 2018 09:40:44 +0100
Subject: dpt_i2o: fix use after free in adpt_release for real

Fixes 7bec5bed ("scsi: dpt_i2o: use after free in adpt_release()")

adpt_i2o_delete_hba still references the host.

---
 drivers/scsi/dpt_i2o.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/scsi/dpt_i2o.c +4 -2
index 0f30792d74c4..35d45903ed2e 100644
@@ -304,10 +304,12 @@ static int adpt_detect(struct scsi_host_template* sht)
 
 static void adpt_release(adpt_hba *pHba)
 {
-	scsi_remove_host(pHba->host);
+	struct Scsi_Host *shost = pHba->host;
+
+	scsi_remove_host(shost);
 //	adpt_i2o_quiesce_hba(pHba);
-	scsi_host_put(pHba->host);
 	adpt_i2o_delete_hba(pHba);
+	scsi_host_put(shost);
 }
 
 
Signature · 1 line
2.14.2
#4 lore raw
In reply to Dan Carpenter

Christoph,

Quoted 3 lines

This fix is broken! adpt_i2o_delete_hba references pHba->host as well.

Instead we need a local variable for the host.

Thanks for spotting this! Fixed it up.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering