Commit aa8f0dc6 authored by Jeff Garzik's avatar Jeff Garzik

libata: Fix use-after-iounmap

Jens Axboe pointed out that the iounmap() call in libata was occurring
too early, and some drivers (ahci, probably others) were using ioremap'd
memory after it had been unmapped.

The patch should address that problem by way of improving the libata
driver API:

* move ->host_stop() call after all ->port_stop() calls have occurred.

* create default helper function ata_host_stop(), and move iounmap()
call there.

* add ->host_stop_prewalk() hook, use it in sata_qstor.c (hi Mark).
sata_qstor appears to require the host-stop-before-port-stop ordering
that existed prior to applying the attached patch.
parent bef9c558
...@@ -289,6 +289,8 @@ static void ahci_host_stop(struct ata_host_set *host_set) ...@@ -289,6 +289,8 @@ static void ahci_host_stop(struct ata_host_set *host_set)
{ {
struct ahci_host_priv *hpriv = host_set->private_data; struct ahci_host_priv *hpriv = host_set->private_data;
kfree(hpriv); kfree(hpriv);
ata_host_stop(host_set);
} }
static int ahci_port_start(struct ata_port *ap) static int ahci_port_start(struct ata_port *ap)
......
...@@ -153,6 +153,7 @@ static struct ata_port_operations piix_pata_ops = { ...@@ -153,6 +153,7 @@ static struct ata_port_operations piix_pata_ops = {
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_operations piix_sata_ops = { static struct ata_port_operations piix_sata_ops = {
...@@ -180,6 +181,7 @@ static struct ata_port_operations piix_sata_ops = { ...@@ -180,6 +181,7 @@ static struct ata_port_operations piix_sata_ops = {
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info piix_port_info[] = { static struct ata_port_info piix_port_info[] = {
......
...@@ -3321,6 +3321,13 @@ void ata_port_stop (struct ata_port *ap) ...@@ -3321,6 +3321,13 @@ void ata_port_stop (struct ata_port *ap)
dma_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma); dma_free_coherent(dev, ATA_PRD_TBL_SZ, ap->prd, ap->prd_dma);
} }
void ata_host_stop (struct ata_host_set *host_set)
{
if (host_set->mmio_base)
iounmap(host_set->mmio_base);
}
/** /**
* ata_host_remove - Unregister SCSI host structure with upper layers * ata_host_remove - Unregister SCSI host structure with upper layers
* @ap: Port to unregister * @ap: Port to unregister
...@@ -3877,10 +3884,6 @@ void ata_pci_remove_one (struct pci_dev *pdev) ...@@ -3877,10 +3884,6 @@ void ata_pci_remove_one (struct pci_dev *pdev)
} }
free_irq(host_set->irq, host_set); free_irq(host_set->irq, host_set);
if (host_set->ops->host_stop)
host_set->ops->host_stop(host_set);
if (host_set->mmio_base)
iounmap(host_set->mmio_base);
for (i = 0; i < host_set->n_ports; i++) { for (i = 0; i < host_set->n_ports; i++) {
ap = host_set->ports[i]; ap = host_set->ports[i];
...@@ -3899,6 +3902,9 @@ void ata_pci_remove_one (struct pci_dev *pdev) ...@@ -3899,6 +3902,9 @@ void ata_pci_remove_one (struct pci_dev *pdev)
scsi_host_put(ap->host); scsi_host_put(ap->host);
} }
if (host_set->ops->host_stop)
host_set->ops->host_stop(host_set);
kfree(host_set); kfree(host_set);
pci_release_regions(pdev); pci_release_regions(pdev);
...@@ -3996,6 +4002,7 @@ EXPORT_SYMBOL_GPL(ata_chk_err); ...@@ -3996,6 +4002,7 @@ EXPORT_SYMBOL_GPL(ata_chk_err);
EXPORT_SYMBOL_GPL(ata_exec_command); EXPORT_SYMBOL_GPL(ata_exec_command);
EXPORT_SYMBOL_GPL(ata_port_start); EXPORT_SYMBOL_GPL(ata_port_start);
EXPORT_SYMBOL_GPL(ata_port_stop); EXPORT_SYMBOL_GPL(ata_port_stop);
EXPORT_SYMBOL_GPL(ata_host_stop);
EXPORT_SYMBOL_GPL(ata_interrupt); EXPORT_SYMBOL_GPL(ata_interrupt);
EXPORT_SYMBOL_GPL(ata_qc_prep); EXPORT_SYMBOL_GPL(ata_qc_prep);
EXPORT_SYMBOL_GPL(ata_bmdma_setup); EXPORT_SYMBOL_GPL(ata_bmdma_setup);
......
...@@ -329,6 +329,8 @@ static void nv_host_stop (struct ata_host_set *host_set) ...@@ -329,6 +329,8 @@ static void nv_host_stop (struct ata_host_set *host_set)
host->host_desc->disable_hotplug(host_set); host->host_desc->disable_hotplug(host_set);
kfree(host); kfree(host);
ata_host_stop(host_set);
} }
static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent) static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
......
...@@ -122,6 +122,7 @@ static struct ata_port_operations pdc_ata_ops = { ...@@ -122,6 +122,7 @@ static struct ata_port_operations pdc_ata_ops = {
.scr_write = pdc_sata_scr_write, .scr_write = pdc_sata_scr_write,
.port_start = pdc_port_start, .port_start = pdc_port_start,
.port_stop = pdc_port_stop, .port_stop = pdc_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info pdc_port_info[] = { static struct ata_port_info pdc_port_info[] = {
......
...@@ -536,6 +536,8 @@ static void qs_host_stop(struct ata_host_set *host_set) ...@@ -536,6 +536,8 @@ static void qs_host_stop(struct ata_host_set *host_set)
writeb(0, mmio_base + QS_HCT_CTRL); /* disable host interrupts */ writeb(0, mmio_base + QS_HCT_CTRL); /* disable host interrupts */
writeb(QS_CNFG3_GSRST, mmio_base + QS_HCF_CNFG3); /* global reset */ writeb(QS_CNFG3_GSRST, mmio_base + QS_HCF_CNFG3); /* global reset */
ata_host_stop(host_set);
} }
static void qs_host_init(unsigned int chip_id, struct ata_probe_ent *pe) static void qs_host_init(unsigned int chip_id, struct ata_probe_ent *pe)
......
...@@ -161,6 +161,7 @@ static struct ata_port_operations sil_ops = { ...@@ -161,6 +161,7 @@ static struct ata_port_operations sil_ops = {
.scr_write = sil_scr_write, .scr_write = sil_scr_write,
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info sil_port_info[] = { static struct ata_port_info sil_port_info[] = {
......
...@@ -114,6 +114,7 @@ static struct ata_port_operations sis_ops = { ...@@ -114,6 +114,7 @@ static struct ata_port_operations sis_ops = {
.scr_write = sis_scr_write, .scr_write = sis_scr_write,
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info sis_port_info = { static struct ata_port_info sis_port_info = {
......
...@@ -313,6 +313,7 @@ static struct ata_port_operations k2_sata_ops = { ...@@ -313,6 +313,7 @@ static struct ata_port_operations k2_sata_ops = {
.scr_write = k2_sata_scr_write, .scr_write = k2_sata_scr_write,
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static void k2_sata_setup_port(struct ata_ioports *port, unsigned long base) static void k2_sata_setup_port(struct ata_ioports *port, unsigned long base)
......
...@@ -245,6 +245,8 @@ static void pdc20621_host_stop(struct ata_host_set *host_set) ...@@ -245,6 +245,8 @@ static void pdc20621_host_stop(struct ata_host_set *host_set)
iounmap(dimm_mmio); iounmap(dimm_mmio);
kfree(hpriv); kfree(hpriv);
ata_host_stop(host_set);
} }
static int pdc_port_start(struct ata_port *ap) static int pdc_port_start(struct ata_port *ap)
......
...@@ -113,6 +113,7 @@ static struct ata_port_operations uli_ops = { ...@@ -113,6 +113,7 @@ static struct ata_port_operations uli_ops = {
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info uli_port_info = { static struct ata_port_info uli_port_info = {
......
...@@ -134,6 +134,7 @@ static struct ata_port_operations svia_sata_ops = { ...@@ -134,6 +134,7 @@ static struct ata_port_operations svia_sata_ops = {
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static struct ata_port_info svia_port_info = { static struct ata_port_info svia_port_info = {
......
...@@ -230,6 +230,7 @@ static struct ata_port_operations vsc_sata_ops = { ...@@ -230,6 +230,7 @@ static struct ata_port_operations vsc_sata_ops = {
.scr_write = vsc_sata_scr_write, .scr_write = vsc_sata_scr_write,
.port_start = ata_port_start, .port_start = ata_port_start,
.port_stop = ata_port_stop, .port_stop = ata_port_stop,
.host_stop = ata_host_stop,
}; };
static void __devinit vsc_sata_setup_port(struct ata_ioports *port, unsigned long base) static void __devinit vsc_sata_setup_port(struct ata_ioports *port, unsigned long base)
......
...@@ -410,6 +410,7 @@ extern u8 ata_chk_err(struct ata_port *ap); ...@@ -410,6 +410,7 @@ extern u8 ata_chk_err(struct ata_port *ap);
extern void ata_exec_command(struct ata_port *ap, struct ata_taskfile *tf); extern void ata_exec_command(struct ata_port *ap, struct ata_taskfile *tf);
extern int ata_port_start (struct ata_port *ap); extern int ata_port_start (struct ata_port *ap);
extern void ata_port_stop (struct ata_port *ap); extern void ata_port_stop (struct ata_port *ap);
extern void ata_host_stop (struct ata_host_set *host_set);
extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs); extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
extern void ata_qc_prep(struct ata_queued_cmd *qc); extern void ata_qc_prep(struct ata_queued_cmd *qc);
extern int ata_qc_issue_prot(struct ata_queued_cmd *qc); extern int ata_qc_issue_prot(struct ata_queued_cmd *qc);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment