ahci: Fix data abort on multiple scsi resets.

Commit 2faf5fb82ed6 introduced a regression that causes a data abort when running scsi init followed by scsi reset. There are 2 problems with the original commit 1) ALLOC_CACHE_ALIGN_BUFFER() allocates memory on the stack but is assigned to ataid[port] and used by other functions. 2) The function ata_scsiop_inquiry() tries to free memory which was never allocated on the heap. Fix these problems by using tmpid as a temporary cache aligned buffer. Allocate memory separately for ataid[port] and re-use it if required. Fixes: 2faf5fb82ed6 (ahci: Fix cache align error messages) Reported-by: Eli Nidam <elini@marvell.com> Signed-off-by: Roger Quadros <rogerq@ti.com>

ahci: Fix data abort on multiple scsi resets.
Commit 2faf5fb82ed6 introduced a regression that causes a data abort when running scsi init followed by scsi reset. There are 2 problems with the original commit 1) ALLOC_CACHE_ALIGN_BUFFER() allocates memory on the stack but is assigned to ataid[port] and used by other functions. 2) The function ata_scsiop_inquiry() tries to free memory which was never allocated on the heap. Fix these problems by using tmpid as a temporary cache aligned buffer. Allocate memory separately for ataid[port] and re-use it if required. Fixes: 2faf5fb82ed6 (ahci: Fix cache align error messages) Reported-by: Eli Nidam <elini@marvell.com> Signed-off-by: Roger Quadros <rogerq@ti.com>
Roger Quadros · Tom Rini
1 parent 1336e2d343
Showing 1 changed file with 16 additions and 7 deletions Side-by-side Diff
drivers/block/ahci.c
@@ -623,6 +623,7 @@
 		95 - 4,
 	};
 	u8 fis[20];
+	u16 *idbuf;
 	ALLOC_CACHE_ALIGN_BUFFER(u16, tmpid, ATA_ID_WORDS);
 	u8 port;
  
  
  
  
@@ -649,17 +650,25 @@
 		return -EIO;
 	}
  
-	if (ataid[port])
-		free(ataid[port]);
-	ataid[port] = tmpid;
-	ata_swap_buf_le16(tmpid, ATA_ID_WORDS);
+	if (!ataid[port]) {
+		ataid[port] = malloc(ATA_ID_WORDS * 2);
+		if (!ataid[port]) {
+			printf("%s: No memory for ataid[port]\n", __func__);
+			return -ENOMEM;
+		}
+	}
  
+	idbuf = ataid[port];
+
+	memcpy(idbuf, tmpid, ATA_ID_WORDS * 2);
+	ata_swap_buf_le16(idbuf, ATA_ID_WORDS);
+
 	memcpy(&pccb->pdata[8], "ATA     ", 8);
-	ata_id_strcpy((u16 *) &pccb->pdata[16], &tmpid[ATA_ID_PROD], 16);
-	ata_id_strcpy((u16 *) &pccb->pdata[32], &tmpid[ATA_ID_FW_REV], 4);
+	ata_id_strcpy((u16 *)&pccb->pdata[16], &idbuf[ATA_ID_PROD], 16);
+	ata_id_strcpy((u16 *)&pccb->pdata[32], &idbuf[ATA_ID_FW_REV], 4);
  
 #ifdef DEBUG
-	ata_dump_id(tmpid);
+	ata_dump_id(idbuf);
 #endif
 	return 0;
 }
...	...	@@ -623,6 +623,7 @@
623	623	95 - 4,
624	624	};
625	625	u8 fis[20];
	626	+ u16 *idbuf;
626	627	ALLOC_CACHE_ALIGN_BUFFER(u16, tmpid, ATA_ID_WORDS);
627	628	u8 port;
628	629
629	630
630	631
631	632
...	...	@@ -649,17 +650,25 @@
649	650	return -EIO;
650	651	}
651	652
652		- if (ataid[port])
653		- free(ataid[port]);
654		- ataid[port] = tmpid;
655		- ata_swap_buf_le16(tmpid, ATA_ID_WORDS);
	653	+ if (!ataid[port]) {
	654	+ ataid[port] = malloc(ATA_ID_WORDS * 2);
	655	+ if (!ataid[port]) {
	656	+ printf("%s: No memory for ataid[port]\n", __func__);
	657	+ return -ENOMEM;
	658	+ }
	659	+ }
656	660
	661	+ idbuf = ataid[port];
	662	+
	663	+ memcpy(idbuf, tmpid, ATA_ID_WORDS * 2);
	664	+ ata_swap_buf_le16(idbuf, ATA_ID_WORDS);
	665	+
657	666	memcpy(&pccb->pdata[8], "ATA ", 8);
658		- ata_id_strcpy((u16 *) &pccb->pdata[16], &tmpid[ATA_ID_PROD], 16);
659		- ata_id_strcpy((u16 *) &pccb->pdata[32], &tmpid[ATA_ID_FW_REV], 4);
	667	+ ata_id_strcpy((u16 *)&pccb->pdata[16], &idbuf[ATA_ID_PROD], 16);
	668	+ ata_id_strcpy((u16 *)&pccb->pdata[32], &idbuf[ATA_ID_FW_REV], 4);
660	669
661	670	#ifdef DEBUG
662		- ata_dump_id(tmpid);
	671	+ ata_dump_id(idbuf);
663	672	#endif
664	673	return 0;
665	674	}