Commit 2fa4a326 authored by Jason Yan's avatar Jason Yan Committed by Martin K. Petersen

scsi: libsas: dynamically allocate and free ata host

Commit 2623c7a5 ("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: b6240a4d ("scsi: libsas: add transport class for ATA devices")
Signed-off-by: default avatarJason 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: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 693ba15c
...@@ -6421,6 +6421,7 @@ void ata_host_init(struct ata_host *host, struct device *dev, ...@@ -6421,6 +6421,7 @@ void ata_host_init(struct ata_host *host, struct device *dev,
host->n_tags = ATA_MAX_QUEUE; host->n_tags = ATA_MAX_QUEUE;
host->dev = dev; host->dev = dev;
host->ops = ops; host->ops = ops;
kref_init(&host->kref);
} }
void __ata_port_probe(struct ata_port *ap) void __ata_port_probe(struct ata_port *ap)
...@@ -7388,3 +7389,5 @@ EXPORT_SYMBOL_GPL(ata_cable_80wire); ...@@ -7388,3 +7389,5 @@ EXPORT_SYMBOL_GPL(ata_cable_80wire);
EXPORT_SYMBOL_GPL(ata_cable_unknown); EXPORT_SYMBOL_GPL(ata_cable_unknown);
EXPORT_SYMBOL_GPL(ata_cable_ignore); EXPORT_SYMBOL_GPL(ata_cable_ignore);
EXPORT_SYMBOL_GPL(ata_cable_sata); EXPORT_SYMBOL_GPL(ata_cable_sata);
EXPORT_SYMBOL_GPL(ata_host_get);
EXPORT_SYMBOL_GPL(ata_host_put);
\ No newline at end of file
...@@ -100,8 +100,6 @@ extern int ata_port_probe(struct ata_port *ap); ...@@ -100,8 +100,6 @@ extern int ata_port_probe(struct ata_port *ap);
extern void __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, extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
u8 page, void *buf, unsigned int sectors); 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) #define to_ata_port(d) container_of(d, struct ata_port, tdev)
......
...@@ -552,34 +552,46 @@ int sas_ata_init(struct domain_device *found_dev) ...@@ -552,34 +552,46 @@ int sas_ata_init(struct domain_device *found_dev)
{ {
struct sas_ha_struct *ha = found_dev->port->ha; struct sas_ha_struct *ha = found_dev->port->ha;
struct Scsi_Host *shost = ha->core.shost; struct Scsi_Host *shost = ha->core.shost;
struct ata_host *ata_host;
struct ata_port *ap; struct ata_port *ap;
int rc; int rc;
ata_host_init(&found_dev->sata_dev.ata_host, ha->dev, &sas_sata_ops); ata_host = kzalloc(sizeof(*ata_host), GFP_KERNEL);
ap = ata_sas_port_alloc(&found_dev->sata_dev.ata_host, if (!ata_host) {
&sata_port_info, SAS_DPRINTK("ata host alloc failed.\n");
shost); 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) { if (!ap) {
SAS_DPRINTK("ata_sas_port_alloc failed.\n"); SAS_DPRINTK("ata_sas_port_alloc failed.\n");
return -ENODEV; rc = -ENODEV;
goto free_host;
} }
ap->private_data = found_dev; ap->private_data = found_dev;
ap->cbl = ATA_CBL_SATA; ap->cbl = ATA_CBL_SATA;
ap->scsi_host = shost; ap->scsi_host = shost;
rc = ata_sas_port_init(ap); rc = ata_sas_port_init(ap);
if (rc) { if (rc)
ata_sas_port_destroy(ap); goto destroy_port;
return rc;
} rc = ata_sas_tport_add(ata_host->dev, ap);
rc = ata_sas_tport_add(found_dev->sata_dev.ata_host.dev, ap); if (rc)
if (rc) { goto destroy_port;
ata_sas_port_destroy(ap);
return rc; found_dev->sata_dev.ata_host = ata_host;
}
found_dev->sata_dev.ap = ap; found_dev->sata_dev.ap = ap;
return 0; 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) void sas_ata_task_abort(struct sas_task *task)
......
...@@ -316,6 +316,8 @@ void sas_free_device(struct kref *kref) ...@@ -316,6 +316,8 @@ void sas_free_device(struct kref *kref)
if (dev_is_sata(dev) && dev->sata_dev.ap) { if (dev_is_sata(dev) && dev->sata_dev.ap) {
ata_sas_tport_delete(dev->sata_dev.ap); ata_sas_tport_delete(dev->sata_dev.ap);
ata_sas_port_destroy(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; dev->sata_dev.ap = NULL;
} }
......
...@@ -1110,6 +1110,8 @@ extern struct ata_host *ata_host_alloc(struct device *dev, int max_ports); ...@@ -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, extern struct ata_host *ata_host_alloc_pinfo(struct device *dev,
const struct ata_port_info * const * ppi, int n_ports); const struct ata_port_info * const * ppi, int n_ports);
extern int ata_slave_link_init(struct ata_port *ap); 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_start(struct ata_host *host);
extern int ata_host_register(struct ata_host *host, extern int ata_host_register(struct ata_host *host,
struct scsi_host_template *sht); struct scsi_host_template *sht);
......
...@@ -161,7 +161,7 @@ struct sata_device { ...@@ -161,7 +161,7 @@ struct sata_device {
u8 port_no; /* port number, if this is a PM (Port) */ u8 port_no; /* port number, if this is a PM (Port) */
struct ata_port *ap; 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 */ struct smp_resp rps_resp ____cacheline_aligned; /* report_phy_sata_resp */
u8 fis[ATA_RESP_FIS_SIZE]; u8 fis[ATA_RESP_FIS_SIZE];
}; };
......
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