scsi: libsas: dynamically allocate and free ata host

Commit 2623c7a5f2 ("libata: add refcounting to ata_host") v4.17+ introduced
refcounting to ata_host and will increase or decrease the refcount when
adding or deleting transport ATA port.

Now the ata host for libsas is embedded in domain_device, and the ->kref
member is not initialized. Afer we add ata transport class, ata_host_get()
will be called when adding transport ATA port and a warning will be
triggered as below:

refcount_t: increment on 0; use-after-free.
WARNING: CPU: 2 PID: 103 at
lib/refcount.c:153 refcount_inc+0x40/0x48 ......  Call trace:
 refcount_inc+0x40/0x48
 ata_host_get+0x10/0x18
 ata_tport_add+0x40/0x120
 ata_sas_tport_add+0xc/0x14
 sas_ata_init+0x7c/0xc8
 sas_discover_domain+0x380/0x53c
 process_one_work+0x12c/0x288
 worker_thread+0x58/0x3f0
 kthread+0xfc/0x128
 ret_from_fork+0x10/0x18

And also when removing transport ATA port ata_host_put() will be called and
another similar warning will be triggered. If the refcount decreased to
zero, the ata host will be freed. But this ata host is only part of
domain_device, it cannot be freed directly.

So we have to change this embedded static ata host to a dynamically
allocated ata host and initialize the ->kref member. To use ata_host_get()
and ata_host_put() in libsas, we need to move the declaration of these
functions to the public libata.h and export them.

Fixes: b6240a4df018 ("scsi: libsas: add transport class for ATA devices")
Signed-off-by: Jason Yan <yanaijie@huawei.com>
CC: John Garry <john.garry@huawei.com>
CC: Taras Kondratiuk <takondra@cisco.com>
CC: Tejun Heo <tj@kernel.org>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 27d15ed7fa3d03771f020cf064749f6f9fe38633..89cb4872a09cbafcdea6e04eaa24cd70287194ad 100644 (file)
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6421,6 +6421,7 @@ void ata_host_init(struct ata_host *host, struct device *dev,
        host->n_tags = ATA_MAX_QUEUE;
        host->dev = dev;
        host->ops = ops;
+       kref_init(&host->kref);
 }
 
 void __ata_port_probe(struct ata_port *ap)
@@ -7388,3 +7389,5 @@ EXPORT_SYMBOL_GPL(ata_cable_80wire);
 EXPORT_SYMBOL_GPL(ata_cable_unknown);
 EXPORT_SYMBOL_GPL(ata_cable_ignore);
 EXPORT_SYMBOL_GPL(ata_cable_sata);
+EXPORT_SYMBOL_GPL(ata_host_get);
+EXPORT_SYMBOL_GPL(ata_host_put);
\ No newline at end of file

diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 9e21c49cf6be8899659f52b177fb3e47b5d49009..f953cb4bb1ba8fe015167d522e08c11b17fa91c7 100644 (file)
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -100,8 +100,6 @@ extern int ata_port_probe(struct ata_port *ap);
 extern void __ata_port_probe(struct ata_port *ap);
 extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
                                      u8 page, void *buf, unsigned int sectors);
-extern void ata_host_get(struct ata_host *host);
-extern void ata_host_put(struct ata_host *host);
 
 #define to_ata_port(d) container_of(d, struct ata_port, tdev)
 

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 2ac7395112b4dd35de3f407177cb494838022a03..64a958a99f6a89896c16354502c34a397d18a1df 100644 (file)
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -552,34 +552,46 @@ int sas_ata_init(struct domain_device *found_dev)
 {
        struct sas_ha_struct *ha = found_dev->port->ha;
        struct Scsi_Host *shost = ha->core.shost;
+       struct ata_host *ata_host;
        struct ata_port *ap;
        int rc;
 
-       ata_host_init(&found_dev->sata_dev.ata_host, ha->dev, &sas_sata_ops);
-       ap = ata_sas_port_alloc(&found_dev->sata_dev.ata_host,
-                               &sata_port_info,
-                               shost);
+       ata_host = kzalloc(sizeof(*ata_host), GFP_KERNEL);
+       if (!ata_host)  {
+               SAS_DPRINTK("ata host alloc failed.\n");
+               return -ENOMEM;
+       }
+
+       ata_host_init(ata_host, ha->dev, &sas_sata_ops);
+
+       ap = ata_sas_port_alloc(ata_host, &sata_port_info, shost);
        if (!ap) {
                SAS_DPRINTK("ata_sas_port_alloc failed.\n");
-               return -ENODEV;
+               rc = -ENODEV;
+               goto free_host;
        }
 
        ap->private_data = found_dev;
        ap->cbl = ATA_CBL_SATA;
        ap->scsi_host = shost;
        rc = ata_sas_port_init(ap);
-       if (rc) {
-               ata_sas_port_destroy(ap);
-               return rc;
-       }
-       rc = ata_sas_tport_add(found_dev->sata_dev.ata_host.dev, ap);
-       if (rc) {
-               ata_sas_port_destroy(ap);
-               return rc;
-       }
+       if (rc)
+               goto destroy_port;
+
+       rc = ata_sas_tport_add(ata_host->dev, ap);
+       if (rc)
+               goto destroy_port;
+
+       found_dev->sata_dev.ata_host = ata_host;
        found_dev->sata_dev.ap = ap;
 
        return 0;
+
+destroy_port:
+       ata_sas_port_destroy(ap);
+free_host:
+       ata_host_put(ata_host);
+       return rc;
 }
 
 void sas_ata_task_abort(struct sas_task *task)

diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index 1ffca28fe6a864f7523a993a1c32cd32140b930b..0148ae62a52a941b154df5181fc80543df4a30cf 100644 (file)
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -316,6 +316,8 @@ void sas_free_device(struct kref *kref)
        if (dev_is_sata(dev) && dev->sata_dev.ap) {
                ata_sas_tport_delete(dev->sata_dev.ap);
                ata_sas_port_destroy(dev->sata_dev.ap);
+               ata_host_put(dev->sata_dev.ata_host);
+               dev->sata_dev.ata_host = NULL;
                dev->sata_dev.ap = NULL;
        }
 

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8b8946dd63b9d4df3d08c5051604fce0fc147be1..33e9718397e22f61d93a7bbb61ffb2d2f43831cf 100644 (file)
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1110,6 +1110,8 @@ extern struct ata_host *ata_host_alloc(struct device *dev, int max_ports);
 extern struct ata_host *ata_host_alloc_pinfo(struct device *dev,
                        const struct ata_port_info * const * ppi, int n_ports);
 extern int ata_slave_link_init(struct ata_port *ap);
+extern void ata_host_get(struct ata_host *host);
+extern void ata_host_put(struct ata_host *host);
 extern int ata_host_start(struct ata_host *host);
 extern int ata_host_register(struct ata_host *host,
                             struct scsi_host_template *sht);

diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 225ab7783dfd44377b6b4ed843b98aa205fdbef1..3de3b10da19a9eee24904a5604bb74744e8eafe9 100644 (file)
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -161,7 +161,7 @@ struct sata_device {
        u8     port_no;        /* port number, if this is a PM (Port) */
 
        struct ata_port *ap;
-       struct ata_host ata_host;
+       struct ata_host *ata_host;
        struct smp_resp rps_resp ____cacheline_aligned; /* report_phy_sata_resp */
        u8     fis[ATA_RESP_FIS_SIZE];
 };