Commit b592211c authored by Mike Snitzer's avatar Mike Snitzer
Browse files

dm mpath: fix attached_handler_name leak and dangling hw_handler_name pointer 



Commit e8f74a0f ("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: e8f74a0f ("dm mpath: eliminate need to use scsi_device_from_queue")
Cc: stable@vger.kernel.org # 4.16+
Reported-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMike Snitzer <snitzer@redhat.com>
parent 013ad043

drivers/md/dm-mpath.c

+8 −6
Original line number Diff line number Diff line
@@ -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 @@ retain: 
			 * 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);

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE