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; | 	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); | 	struct ata_host *host = dev_get_drvdata(gendev); | ||||||
| 	int i; | 	int i; | ||||||
|  | @ -6019,13 +6019,36 @@ static void ata_host_release(struct device *gendev, void *res) | ||||||
| 		if (ap->scsi_host) | 		if (ap->scsi_host) | ||||||
| 			scsi_host_put(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->pmp_link); | ||||||
| 		kfree(ap->slave_link); | 		kfree(ap->slave_link); | ||||||
| 		kfree(ap); | 		kfree(ap); | ||||||
| 		host->ports[i] = NULL; | 		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; | 	struct ata_host *host; | ||||||
| 	size_t sz; | 	size_t sz; | ||||||
| 	int i; | 	int i; | ||||||
|  | 	void *dr; | ||||||
| 
 | 
 | ||||||
| 	DPRINTK("ENTER\n"); | 	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)) | 	if (!devres_open_group(dev, NULL, GFP_KERNEL)) | ||||||
| 		return NULL; | 		return NULL; | ||||||
| 
 | 
 | ||||||
| 	/* alloc a container for our list of ATA ports (buses) */ | 	dr = devres_alloc(ata_devres_release, 0, GFP_KERNEL); | ||||||
| 	sz = sizeof(struct ata_host) + (max_ports + 1) * sizeof(void *); | 	if (!dr) | ||||||
| 	/* alloc a container for our list of ATA ports (buses) */ |  | ||||||
| 	host = devres_alloc(ata_host_release, sz, GFP_KERNEL); |  | ||||||
| 	if (!host) |  | ||||||
| 		goto err_out; | 		goto err_out; | ||||||
| 
 | 
 | ||||||
| 	devres_add(dev, host); | 	devres_add(dev, dr); | ||||||
| 	dev_set_drvdata(dev, host); | 	dev_set_drvdata(dev, host); | ||||||
| 
 | 
 | ||||||
| 	spin_lock_init(&host->lock); | 	spin_lock_init(&host->lock); | ||||||
| 	mutex_init(&host->eh_mutex); | 	mutex_init(&host->eh_mutex); | ||||||
| 	host->dev = dev; | 	host->dev = dev; | ||||||
| 	host->n_ports = max_ports; | 	host->n_ports = max_ports; | ||||||
|  | 	kref_init(&host->kref); | ||||||
| 
 | 
 | ||||||
| 	/* allocate ports bound to this host */ | 	/* allocate ports bound to this host */ | ||||||
| 	for (i = 0; i < max_ports; i++) { | 	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) | 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->type = &ata_port_type; | ||||||
| 
 | 
 | ||||||
| 	dev->parent = parent; | 	dev->parent = parent; | ||||||
|  | 	ata_host_get(ap->host); | ||||||
| 	dev->release = ata_tport_release; | 	dev->release = ata_tport_release; | ||||||
| 	dev_set_name(dev, "ata%d", ap->print_id); | 	dev_set_name(dev, "ata%d", ap->print_id); | ||||||
| 	transport_setup_device(dev); | 	transport_setup_device(dev); | ||||||
|  | @ -314,6 +317,7 @@ int ata_tport_add(struct device *parent, | ||||||
|  tport_err: |  tport_err: | ||||||
| 	transport_destroy_device(dev); | 	transport_destroy_device(dev); | ||||||
| 	put_device(dev); | 	put_device(dev); | ||||||
|  | 	ata_host_put(ap->host); | ||||||
| 	return error; | 	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 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) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -617,6 +617,7 @@ struct ata_host { | ||||||
| 	void			*private_data; | 	void			*private_data; | ||||||
| 	struct ata_port_operations *ops; | 	struct ata_port_operations *ops; | ||||||
| 	unsigned long		flags; | 	unsigned long		flags; | ||||||
|  | 	struct kref		kref; | ||||||
| 
 | 
 | ||||||
| 	struct mutex		eh_mutex; | 	struct mutex		eh_mutex; | ||||||
| 	struct task_struct	*eh_owner; | 	struct task_struct	*eh_owner; | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Taras Kondratiuk
						Taras Kondratiuk