forked from mirrors/linux
		
	libata: add refcounting to ata_host
After commit9a6d6a2dda("ata: make ata port as parent device of scsi host") manual driver unbind/remove causes use-after-free. Unbind unconditionally invokes devres_release_all() which calls ata_host_release() and frees ata_host/ata_port memory while it is still being referenced as a parent of SCSI host. When SCSI host is finally released scsi_host_dev_release() calls put_device(parent) and accesses freed ata_port memory. Add reference counting to make sure that ata_host lives long enough. Bug report: https://lkml.org/lkml/2017/11/1/945 Fixes:9a6d6a2dda("ata: make ata port as parent device of scsi host") Cc: Tejun Heo <tj@kernel.org> Cc: Lin Ming <minggr@gmail.com> Cc: linux-ide@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Taras Kondratiuk <takondra@cisco.com> Signed-off-by: Tejun Heo <tj@kernel.org>
This commit is contained in:
		
							parent
							
								
									a80ea4cb94
								
							
						
					
					
						commit
						2623c7a5f2
					
				
					 4 changed files with 43 additions and 8 deletions
				
			
		|  | @ -6005,7 +6005,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host) | |||
| 	return ap; | ||||
| } | ||||
| 
 | ||||
| static void ata_host_release(struct device *gendev, void *res) | ||||
| static void ata_devres_release(struct device *gendev, void *res) | ||||
| { | ||||
| 	struct ata_host *host = dev_get_drvdata(gendev); | ||||
| 	int i; | ||||
|  | @ -6019,13 +6019,36 @@ static void ata_host_release(struct device *gendev, void *res) | |||
| 		if (ap->scsi_host) | ||||
| 			scsi_host_put(ap->scsi_host); | ||||
| 
 | ||||
| 	} | ||||
| 
 | ||||
| 	dev_set_drvdata(gendev, NULL); | ||||
| 	ata_host_put(host); | ||||
| } | ||||
| 
 | ||||
| static void ata_host_release(struct kref *kref) | ||||
| { | ||||
| 	struct ata_host *host = container_of(kref, struct ata_host, kref); | ||||
| 	int i; | ||||
| 
 | ||||
| 	for (i = 0; i < host->n_ports; i++) { | ||||
| 		struct ata_port *ap = host->ports[i]; | ||||
| 
 | ||||
| 		kfree(ap->pmp_link); | ||||
| 		kfree(ap->slave_link); | ||||
| 		kfree(ap); | ||||
| 		host->ports[i] = NULL; | ||||
| 	} | ||||
| 	kfree(host); | ||||
| } | ||||
| 
 | ||||
| 	dev_set_drvdata(gendev, NULL); | ||||
| void ata_host_get(struct ata_host *host) | ||||
| { | ||||
| 	kref_get(&host->kref); | ||||
| } | ||||
| 
 | ||||
| void ata_host_put(struct ata_host *host) | ||||
| { | ||||
| 	kref_put(&host->kref, ata_host_release); | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  | @ -6053,26 +6076,31 @@ struct ata_host *ata_host_alloc(struct device *dev, int max_ports) | |||
| 	struct ata_host *host; | ||||
| 	size_t sz; | ||||
| 	int i; | ||||
| 	void *dr; | ||||
| 
 | ||||
| 	DPRINTK("ENTER\n"); | ||||
| 
 | ||||
| 	/* alloc a container for our list of ATA ports (buses) */ | ||||
| 	sz = sizeof(struct ata_host) + (max_ports + 1) * sizeof(void *); | ||||
| 	host = kzalloc(sz, GFP_KERNEL); | ||||
| 	if (!host) | ||||
| 		return NULL; | ||||
| 
 | ||||
| 	if (!devres_open_group(dev, NULL, GFP_KERNEL)) | ||||
| 		return NULL; | ||||
| 
 | ||||
| 	/* alloc a container for our list of ATA ports (buses) */ | ||||
| 	sz = sizeof(struct ata_host) + (max_ports + 1) * sizeof(void *); | ||||
| 	/* alloc a container for our list of ATA ports (buses) */ | ||||
| 	host = devres_alloc(ata_host_release, sz, GFP_KERNEL); | ||||
| 	if (!host) | ||||
| 	dr = devres_alloc(ata_devres_release, 0, GFP_KERNEL); | ||||
| 	if (!dr) | ||||
| 		goto err_out; | ||||
| 
 | ||||
| 	devres_add(dev, host); | ||||
| 	devres_add(dev, dr); | ||||
| 	dev_set_drvdata(dev, host); | ||||
| 
 | ||||
| 	spin_lock_init(&host->lock); | ||||
| 	mutex_init(&host->eh_mutex); | ||||
| 	host->dev = dev; | ||||
| 	host->n_ports = max_ports; | ||||
| 	kref_init(&host->kref); | ||||
| 
 | ||||
| 	/* allocate ports bound to this host */ | ||||
| 	for (i = 0; i < max_ports; i++) { | ||||
|  |  | |||
|  | @ -224,6 +224,8 @@ static DECLARE_TRANSPORT_CLASS(ata_port_class, | |||
| 
 | ||||
| static void ata_tport_release(struct device *dev) | ||||
| { | ||||
| 	struct ata_port *ap = tdev_to_port(dev); | ||||
| 	ata_host_put(ap->host); | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  | @ -284,6 +286,7 @@ int ata_tport_add(struct device *parent, | |||
| 	dev->type = &ata_port_type; | ||||
| 
 | ||||
| 	dev->parent = parent; | ||||
| 	ata_host_get(ap->host); | ||||
| 	dev->release = ata_tport_release; | ||||
| 	dev_set_name(dev, "ata%d", ap->print_id); | ||||
| 	transport_setup_device(dev); | ||||
|  | @ -314,6 +317,7 @@ int ata_tport_add(struct device *parent, | |||
|  tport_err: | ||||
| 	transport_destroy_device(dev); | ||||
| 	put_device(dev); | ||||
| 	ata_host_put(ap->host); | ||||
| 	return error; | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -100,6 +100,8 @@ 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) | ||||
| 
 | ||||
|  |  | |||
|  | @ -617,6 +617,7 @@ struct ata_host { | |||
| 	void			*private_data; | ||||
| 	struct ata_port_operations *ops; | ||||
| 	unsigned long		flags; | ||||
| 	struct kref		kref; | ||||
| 
 | ||||
| 	struct mutex		eh_mutex; | ||||
| 	struct task_struct	*eh_owner; | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Taras Kondratiuk
						Taras Kondratiuk