libata fixes

PATCH In review Jeff Garzik 2 participants 3 messages Dec 5, 2003 SCSI

lore

Share & tools

lore

Jeff Garzik

author

2003-12-05 18:17 UTC #0 lore raw

PATCH 1/1 libata fixes

Linus, please do a

bk pull bk://gkernel.bkbits.net/libata-2.5

This will update the following files:

 drivers/scsi/libata-core.c  |   17 ++---
 drivers/scsi/sata_promise.c |  128 +++++++++++++++++++++++++-------------------
 2 files changed, 81 insertions(+), 64 deletions(-)

through these ChangeSets:

<[email protected]> (03/12/05 1.1498)
[libata] fix use-after-free

Fixes oops some were seeing on module unload.

Caught by Jon Burgess.

<[email protected]> (03/11/30 1.1489.3.2)
[libata] use sg_dma_xxx macros

Fixes build on some platforms, fixes issues on others.

<[email protected]> (03/11/26 1.1489.3.1)
[libata] Fix PDC20621: we only have one Host DMA engine, not one per port

Whoops. So, we need to queue HDMA transactions internally.

drivers/scsi/libata-core.c +8 -9

diff -Nru a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
@@ -1635,8 +1635,8 @@
 	if (cmd->use_sg)
 		pci_unmap_sg(ap->host_set->pdev, sg, qc->n_elem, dir);
 	else
-		pci_unmap_single(ap->host_set->pdev, sg[0].dma_address,
-				 sg[0].length, dir);
+		pci_unmap_single(ap->host_set->pdev, sg_dma_address(&sg[0]),
+				 sg_dma_len(&sg[0]), dir);
 
 	qc->flags &= ~ATA_QCFLAG_SG;
 	qc->sg = NULL;

@@ -1659,8 +1659,8 @@
 	assert(qc->n_elem > 0);
 
 	for (i = 0; i < qc->n_elem; i++) {
-		ap->prd[i].addr = cpu_to_le32(sg[i].dma_address);
-		ap->prd[i].flags_len = cpu_to_le32(sg[i].length);
+		ap->prd[i].addr = cpu_to_le32(sg_dma_address(&sg[i]));
+		ap->prd[i].flags_len = cpu_to_le32(sg_dma_len(&sg[i]));
 		VPRINTK("PRD[%u] = (0x%X, 0x%X)\n",
 			i, le32_to_cpu(ap->prd[i].addr), le32_to_cpu(ap->prd[i].flags_len));
 	}

@@ -1691,12 +1691,12 @@
 
 	sg->page = virt_to_page(cmd->request_buffer);
 	sg->offset = (unsigned long) cmd->request_buffer & ~PAGE_MASK;
-	sg->length = cmd->request_bufflen;
+	sg_dma_len(sg) = cmd->request_bufflen;
 
 	if (!have_sg)
 		return 0;
 
-	sg->dma_address = pci_map_single(ap->host_set->pdev,
+	sg_dma_address(sg) = pci_map_single(ap->host_set->pdev,
 					 cmd->request_buffer,
 					 cmd->request_bufflen, dir);

@@ -1917,7 +1917,7 @@
 	qc->cursg_ofs++;
 
 	if (cmd->use_sg)
-		if ((qc->cursg_ofs * ATA_SECT_SIZE) == sg[qc->cursg].length) {
+		if ((qc->cursg_ofs * ATA_SECT_SIZE) == sg_dma_len(&sg[qc->cursg])) {
 			qc->cursg++;
 			qc->cursg_ofs = 0;
 		}

@@ -3224,8 +3224,6 @@
 		scsi_host_put(ap->host); /* FIXME: check return val */
 	}
 
-	kfree(host_set);
-
 	pci_release_regions(pdev);
 
 	for (i = 0; i < host_set->n_ports; i++) {

@@ -3242,6 +3240,7 @@
 		}
 	}
 
+	kfree(host_set);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 }

drivers/scsi/sata_promise.c +73 ~~-55~~

201 lines not on this page · raw message

Linus Torvalds

2003-12-05 18:46 UTC #1 lore raw

Quoted 9 lines · Jeff Garzik

Linus, please do a
bk pull bk://gkernel.bkbits.net/libata-2.5
This will update the following files:
 drivers/scsi/libata-core.c  |   17 ++---
 drivers/scsi/sata_promise.c |  128 +++++++++++++++++++++++++-------------------

Right now, I'm accepting one-liners that I think are "obvious" and also "very important" (ie fixes for oopses that anybody can trigger, rather than for example updates to one particular driver). So it sounds like I might accept _one_ of these:

Quoted 6 lines

<[email protected]> (03/12/05 1.1498)
[libata] fix use-after-free
Fixes oops some were seeing on module unload.
Caught by Jon Burgess.

If this is basically an obvious one-liner ("move a kfree")?

Andrew is still off, and he can make a decision independently, but right now I'm not going to apply anything bigger.

Linus

Jeff Garzik

author

2003-12-05 18:49 UTC #2 lore raw

Quoted 24 lines · Linus Torvalds

Quoted 8 lines · Jeff Garzik
Linus, please do a
bk pull bk://gkernel.bkbits.net/libata-2.5
This will update the following files:
drivers/scsi/libata-core.c | 17 ++---
drivers/scsi/sata_promise.c | 128 +++++++++++++++++++++++++-------------------
Right now, I'm accepting one-liners that I think are "obvious" and also "very important" (ie fixes for oopses that anybody can trigger, rather than for example updates to one particular driver). So it sounds like I might accept _one_ of these:
Quoted 5 lines
<[email protected]> (03/12/05 1.1498)
[libata] fix use-after-free
Fixes oops some were seeing on module unload.

Quoted 7 lines

Quoted 1 line
Caught by Jon Burgess.
If this is basically an obvious one-liner ("move a kfree")?
Andrew is still off, and he can make a decision independently, but right now I'm not going to apply anything bigger.

Yep, split out and attached...

Jeff

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.1497 -> 1.1498
# drivers/scsi/libata-core.c 1.8 -> 1.9
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/12/05 [email protected] 1.1498
# [libata] fix use-after-free
#
# Fixes oops some were seeing on module unload.
#
# Caught by Jon Burgess.
# --------------------------------------------
#

drivers/scsi/libata-core.c +1 -2

diff -Nru a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
@@ -3224,8 +3224,6 @@
 		scsi_host_put(ap->host); /* FIXME: check return val */
 	}
 
-	kfree(host_set);
-
 	pci_release_regions(pdev);
 
 	for (i = 0; i < host_set->n_ports; i++) {

@@ -3242,6 +3240,7 @@
 		}
 	}
 
+	kfree(host_set);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 }