From 2fa4a32613c9182b00e46872755b0662374424a7 Mon Sep 17 00:00:00 2001 From: Jason Yan Date: Thu, 10 May 2018 11:05:16 +0800 Subject: 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 CC: John Garry CC: Taras Kondratiuk CC: Tejun Heo Acked-by: Tejun Heo Signed-off-by: Martin K. Petersen --- include/scsi/libsas.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/scsi') diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h index 225ab7783dfd..3de3b10da19a 100644 --- 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]; }; -- cgit v1.2.3-55-g7522 From c84b023a4c1461498abf0eda54f60e2fd64a1ca2 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sun, 24 Jun 2018 22:03:26 +0800 Subject: scsi: read host_busy via scsi_host_busy() No functional change. Just introduce scsi_host_busy() and replace the direct read of scsi_host->host_busy with this new API. Cc: Omar Sandoval , Cc: "Martin K. Petersen" , Cc: James Bottomley , Cc: Christoph Hellwig , Cc: Don Brace Cc: Kashyap Desai Cc: Mike Snitzer Cc: Hannes Reinecke Cc: Laurence Oberman Cc: Bart Van Assche Signed-off-by: Ming Lei Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/advansys.c | 8 ++++---- drivers/scsi/hosts.c | 10 ++++++++++ drivers/scsi/libsas/sas_scsi_host.c | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++-- drivers/scsi/qlogicpti.c | 2 +- drivers/scsi/scsi.c | 2 +- drivers/scsi/scsi_error.c | 6 +++--- drivers/scsi/scsi_sysfs.c | 2 +- include/scsi/scsi_host.h | 1 + 10 files changed, 26 insertions(+), 15 deletions(-) (limited to 'include/scsi') diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c index c9a52905070e..713f69033f20 100644 --- a/drivers/scsi/advansys.c +++ b/drivers/scsi/advansys.c @@ -2416,8 +2416,8 @@ static void asc_prt_scsi_host(struct Scsi_Host *s) struct asc_board *boardp = shost_priv(s); printk("Scsi_Host at addr 0x%p, device %s\n", s, dev_name(boardp->dev)); - printk(" host_busy %u, host_no %d,\n", - atomic_read(&s->host_busy), s->host_no); + printk(" host_busy %d, host_no %d,\n", + scsi_host_busy(s), s->host_no); printk(" base 0x%lx, io_port 0x%lx, irq %d,\n", (ulong)s->base, (ulong)s->io_port, boardp->irq); @@ -3182,8 +3182,8 @@ static void asc_prt_driver_conf(struct seq_file *m, struct Scsi_Host *shost) shost->host_no); seq_printf(m, - " host_busy %u, max_id %u, max_lun %llu, max_channel %u\n", - atomic_read(&shost->host_busy), shost->max_id, + " host_busy %d, max_id %u, max_lun %llu, max_channel %u\n", + scsi_host_busy(shost), shost->max_id, shost->max_lun, shost->max_channel); seq_printf(m, diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 3771e59a9fae..ea4b0bb0c1cd 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -563,6 +563,16 @@ struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost) } EXPORT_SYMBOL(scsi_host_get); +/** + * scsi_host_busy - Return the host busy counter + * @shost: Pointer to Scsi_Host to inc. + **/ +int scsi_host_busy(struct Scsi_Host *shost) +{ + return atomic_read(&shost->host_busy); +} +EXPORT_SYMBOL(scsi_host_busy); + /** * scsi_host_put - dec a Scsi_Host ref count * @shost: Pointer to Scsi_Host to dec. diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index ceab5e5c41c2..33229348dcb6 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -759,7 +759,7 @@ retry: spin_unlock_irq(shost->host_lock); SAS_DPRINTK("Enter %s busy: %d failed: %d\n", - __func__, atomic_read(&shost->host_busy), shost->host_failed); + __func__, scsi_host_busy(shost), shost->host_failed); /* * Deal with commands that still have SAS tasks (i.e. they didn't * complete via the normal sas_task completion mechanism), @@ -801,7 +801,7 @@ out: goto retry; SAS_DPRINTK("--- Exit %s: busy: %d failed: %d tries: %d\n", - __func__, atomic_read(&shost->host_busy), + __func__, scsi_host_busy(shost), shost->host_failed, tries); } diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index e6ba02793610..9aa9590c5373 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -2834,7 +2834,7 @@ static int megasas_reset_bus_host(struct scsi_cmnd *scmd) "SCSI command pointer: (%p)\t SCSI host state: %d\t" " SCSI host busy: %d\t FW outstanding: %d\n", scmd, scmd->device->host->shost_state, - atomic_read((atomic_t *)&scmd->device->host->host_busy), + scsi_host_busy(scmd->device->host), atomic_read(&instance->fw_outstanding)); /* diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 2053970fc9f8..dc41bd3de08a 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3250,7 +3250,7 @@ _base_recovery_check(struct MPT3SAS_ADAPTER *ioc) * See _wait_for_commands_to_complete() call with regards to this code. */ if (ioc->shost_recovery && ioc->pending_io_count) { - ioc->pending_io_count = atomic_read(&ioc->shost->host_busy); + ioc->pending_io_count = scsi_host_busy(ioc->shost); if (ioc->pending_io_count == 0) wake_up(&ioc->reset_wq); } @@ -6857,7 +6857,7 @@ mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc) return; /* pending command count */ - ioc->pending_io_count = atomic_read(&ioc->shost->host_busy); + ioc->pending_io_count = scsi_host_busy(ioc->shost); if (!ioc->pending_io_count) return; diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c index 8578e566ab41..9d09228eee28 100644 --- a/drivers/scsi/qlogicpti.c +++ b/drivers/scsi/qlogicpti.c @@ -959,7 +959,7 @@ static inline void update_can_queue(struct Scsi_Host *host, u_int in_ptr, u_int /* Temporary workaround until bug is found and fixed (one bug has been found already, but fixing it makes things even worse) -jj */ int num_free = QLOGICPTI_REQ_QUEUE_LEN - REQ_QUEUE_DEPTH(in_ptr, out_ptr) - 64; - host->can_queue = atomic_read(&host->host_busy) + num_free; + host->can_queue = scsi_host_busy(host) + num_free; host->sg_tablesize = QLOGICPTI_MAX_SG(num_free); } diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 70ef3c39061d..fc1356d101b0 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -167,7 +167,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) if (level > 3) scmd_printk(KERN_INFO, cmd, "scsi host busy %d failed %d\n", - atomic_read(&cmd->device->host->host_busy), + scsi_host_busy(cmd->device->host), cmd->device->host->host_failed); } } diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 8932ae81a15a..6a014fd15fe9 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -66,7 +66,7 @@ void scsi_eh_wakeup(struct Scsi_Host *shost) { lockdep_assert_held(shost->host_lock); - if (atomic_read(&shost->host_busy) == shost->host_failed) { + if (scsi_host_busy(shost) == shost->host_failed) { trace_scsi_eh_wakeup(shost); wake_up_process(shost->ehandler); SCSI_LOG_ERROR_RECOVERY(5, shost_printk(KERN_INFO, shost, @@ -2155,7 +2155,7 @@ int scsi_error_handler(void *data) break; if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) || - shost->host_failed != atomic_read(&shost->host_busy)) { + shost->host_failed != scsi_host_busy(shost)) { SCSI_LOG_ERROR_RECOVERY(1, shost_printk(KERN_INFO, shost, "scsi_eh_%d: sleeping\n", @@ -2170,7 +2170,7 @@ int scsi_error_handler(void *data) "scsi_eh_%d: waking up %d/%d/%d\n", shost->host_no, shost->host_eh_scheduled, shost->host_failed, - atomic_read(&shost->host_busy))); + scsi_host_busy(shost))); /* * We have a host that is failing for some reason. Figure out diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 7943b762c12d..de122354d09a 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -382,7 +382,7 @@ static ssize_t show_host_busy(struct device *dev, struct device_attribute *attr, char *buf) { struct Scsi_Host *shost = class_to_shost(dev); - return snprintf(buf, 20, "%d\n", atomic_read(&shost->host_busy)); + return snprintf(buf, 20, "%d\n", scsi_host_busy(shost)); } static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL); diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 53b485fe9b67..5ea06d310a25 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -758,6 +758,7 @@ extern void scsi_scan_host(struct Scsi_Host *); extern void scsi_rescan_device(struct device *); extern void scsi_remove_host(struct Scsi_Host *); extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *); +extern int scsi_host_busy(struct Scsi_Host *shost); extern void scsi_host_put(struct Scsi_Host *t); extern struct Scsi_Host *scsi_host_lookup(unsigned short); extern const char *scsi_host_state_name(enum scsi_host_state); -- cgit v1.2.3-55-g7522