nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

dm mpath: fix attached_handler_name leak and dangling hw_handler_name pointer

Browse source 
Commit e8f74a0f00 ("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:
Mike Snitzer 2018-09-17 11:38:47 -04:00
parent 013ad04390
commit b592211c33
1 changed files with 8 additions and 6 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

14
drivers/md/dm-mpath.c
View file

@ -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, 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); struct request_queue *q = bdev_get_queue(bdev);
int r; int r;
if (test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags)) { if (test_bit(MPATHF_RETAIN_ATTACHED_HW_HANDLER, &m->flags)) {
retain: retain:
if (attached_handler_name) { if (*attached_handler_name) {
/* /*
* Clear any hw_handler_params associated with a * Clear any hw_handler_params associated with a
* handler that isn't already attached. * 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); kfree(m->hw_handler_params);
m->hw_handler_params = NULL; 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. * handler instead of the original table passed in.
*/ */
kfree(m->hw_handler_name); 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 pgpath *p;
struct multipath *m = ti->private; struct multipath *m = ti->private;
struct request_queue *q; struct request_queue *q;
const char *attached_handler_name; const char *attached_handler_name = NULL;
/* we need at least a path arg */ /* we need at least a path arg */
if (as->argc < 1) { 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); attached_handler_name = scsi_dh_attached_handler_name(q, GFP_KERNEL);
if (attached_handler_name || m->hw_handler_name) { if (attached_handler_name || m->hw_handler_name) {
INIT_DELAYED_WORK(&p->activate_path, activate_path_work); 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) { if (r) {
dm_put_device(ti, p->path.dev); dm_put_device(ti, p->path.dev);
goto bad; goto bad;
@ -905,6 +906,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
return p; return p;
bad: bad:
kfree(attached_handler_name);
free_pgpath(p); free_pgpath(p);
return ERR_PTR(r); return ERR_PTR(r);
} }