forked from mirrors/linux
		
	dm mpath: fix attached_handler_name leak and dangling hw_handler_name pointer
Commite8f74a0f00("dm mpath: eliminate need to use scsi_device_from_queue") introduced 2 regressions: 1) memory leak occurs if attached_handler_name is not assigned to m->hw_handler_name 2) m->hw_handler_name can become a dangling pointer if the RETAIN_ATTACHED_HW_HANDLER flag is set and scsi_dh_attach() returns -EBUSY. Fix both of these by clearing 'attached_handler_name' pointer passed to setup_scsi_dh() after it is assigned to m->hw_handler_name. And if setup_scsi_dh() doesn't consume 'attached_handler_name' parse_path() will kfree() it. Fixes:e8f74a0f00("dm mpath: eliminate need to use scsi_device_from_queue") Cc: stable@vger.kernel.org # 4.16+ Reported-by: Bart Van Assche <bvanassche@acm.org> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
This commit is contained in:
		
							parent
							
								
									013ad04390
								
							
						
					
					
						commit
						b592211c33
					
				
					 1 changed files with 8 additions and 6 deletions
				
			
		|  | @ -806,19 +806,19 @@ static int parse_path_selector(struct dm_arg_set *as, struct priority_group *pg, | |||
| } | ||||
| 
 | ||||
| static int setup_scsi_dh(struct block_device *bdev, struct multipath *m, | ||||
| 			 const char *attached_handler_name, char **error) | ||||
| 			 const char **attached_handler_name, char **error) | ||||
| { | ||||
| 	struct request_queue *q = bdev_get_queue(bdev); | ||||
| 	int r; | ||||
| 
 | ||||
| 	if (test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags)) { | ||||
| retain: | ||||
| 		if (attached_handler_name) { | ||||
| 		if (*attached_handler_name) { | ||||
| 			/*
 | ||||
| 			 * Clear any hw_handler_params associated with a | ||||
| 			 * handler that isn't already attached. | ||||
| 			 */ | ||||
| 			if (m->hw_handler_name && strcmp(attached_handler_name, m->hw_handler_name)) { | ||||
| 			if (m->hw_handler_name && strcmp(*attached_handler_name, m->hw_handler_name)) { | ||||
| 				kfree(m->hw_handler_params); | ||||
| 				m->hw_handler_params = NULL; | ||||
| 			} | ||||
|  | @ -830,7 +830,8 @@ static int setup_scsi_dh(struct block_device *bdev, struct multipath *m, | |||
| 			 * handler instead of the original table passed in. | ||||
| 			 */ | ||||
| 			kfree(m->hw_handler_name); | ||||
| 			m->hw_handler_name = attached_handler_name; | ||||
| 			m->hw_handler_name = *attached_handler_name; | ||||
| 			*attached_handler_name = NULL; | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
|  | @ -867,7 +868,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps | |||
| 	struct pgpath *p; | ||||
| 	struct multipath *m = ti->private; | ||||
| 	struct request_queue *q; | ||||
| 	const char *attached_handler_name; | ||||
| 	const char *attached_handler_name = NULL; | ||||
| 
 | ||||
| 	/* we need at least a path arg */ | ||||
| 	if (as->argc < 1) { | ||||
|  | @ -890,7 +891,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps | |||
| 	attached_handler_name = scsi_dh_attached_handler_name(q, GFP_KERNEL); | ||||
| 	if (attached_handler_name || m->hw_handler_name) { | ||||
| 		INIT_DELAYED_WORK(&p->activate_path, activate_path_work); | ||||
| 		r = setup_scsi_dh(p->path.dev->bdev, m, attached_handler_name, &ti->error); | ||||
| 		r = setup_scsi_dh(p->path.dev->bdev, m, &attached_handler_name, &ti->error); | ||||
| 		if (r) { | ||||
| 			dm_put_device(ti, p->path.dev); | ||||
| 			goto bad; | ||||
|  | @ -905,6 +906,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps | |||
| 
 | ||||
| 	return p; | ||||
|  bad: | ||||
| 	kfree(attached_handler_name); | ||||
| 	free_pgpath(p); | ||||
| 	return ERR_PTR(r); | ||||
| } | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Mike Snitzer
						Mike Snitzer