| Message ID | 20151117093654.GA13022@xzibit.linux.bs1.fc.nec.co.jp (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
On 11/17/2015 01:36 AM, Junichi Nomura wrote: > In multipath_prepare_ioctl(), > - pgpath is a path selected from available paths > - m->queue_io is true if we cannot send a request immediately to > paths, either because: > * there is no available path > * the path group needs activation (pg_init) > - pg_init is not started > - pg_init is still running > - m->queue_if_no_path is true if the device is configured to queue > I/O if there is no available path > > If !pgpath && !m->queue_if_no_path, the handler should return -EIO. > However in the course of refactoring the condition check has broken > and returns success in that case. Since bdev points to the dm device > itself, dm_blk_ioctl() calls __blk_dev_driver_ioctl() for itself and > recurses until crash. > > You could reproduce the problem like this: > > # dmsetup create mp --table '0 1024 multipath 0 0 0 0' > # sg_inq /dev/mapper/mp > <crash> > [ 172.648615] BUG: unable to handle kernel paging request at fffffffc81b10268 > [ 172.662843] PGD 19dd067 PUD 0 > [ 172.666269] Thread overran stack, or stack corrupted > [ 172.671808] Oops: 0000 [#1] SMP > ... > > This patch fixes the condition check with some clarifications. > > Fixes: e56f81e0b01e ("dm: refactor ioctl handling") > Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Mike Snitzer <snitzer@redhat.com> Since I was able to reproduce this crash and since I haven't seen that crash anymore after I had applied this patch, Tested-by: Bart Van Assche <bart.vanassche@sandisk.com> -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Tue, Nov 17 2015 at 4:36am -0500, Junichi Nomura <j-nomura@ce.jp.nec.com> wrote: > In multipath_prepare_ioctl(), > - pgpath is a path selected from available paths > - m->queue_io is true if we cannot send a request immediately to > paths, either because: > * there is no available path > * the path group needs activation (pg_init) > - pg_init is not started > - pg_init is still running > - m->queue_if_no_path is true if the device is configured to queue > I/O if there is no available path > > If !pgpath && !m->queue_if_no_path, the handler should return -EIO. > However in the course of refactoring the condition check has broken > and returns success in that case. Since bdev points to the dm device > itself, dm_blk_ioctl() calls __blk_dev_driver_ioctl() for itself and > recurses until crash. > > You could reproduce the problem like this: > > # dmsetup create mp --table '0 1024 multipath 0 0 0 0' > # sg_inq /dev/mapper/mp > <crash> > [ 172.648615] BUG: unable to handle kernel paging request at fffffffc81b10268 > [ 172.662843] PGD 19dd067 PUD 0 > [ 172.666269] Thread overran stack, or stack corrupted > [ 172.671808] Oops: 0000 [#1] SMP > ... > > This patch fixes the condition check with some clarifications. > > Fixes: e56f81e0b01e ("dm: refactor ioctl handling") > Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> > Cc: Christoph Hellwig <hch@lst.de> > Cc: Mike Snitzer <snitzer@redhat.com> I've staged this fix for 4.4-rc, see: https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.4&id=43e43c9ea60a7a1831ec823773e924d2dadefd44 I think your fix improves the readability of the code. But I also applied this fix based on the above patch header (which would also resolve this issue without your fix): https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.4&id=647a20d5cad7477033bc021ec9dd75edf4bbf9a0 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index aaa6caa..0ad50ff 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -1537,29 +1537,31 @@ static int multipath_prepare_ioctl(struct dm_target *ti, struct block_device **bdev, fmode_t *mode) { struct multipath *m = ti->private; - struct pgpath *pgpath; unsigned long flags; int r; - r = 0; - spin_lock_irqsave(&m->lock, flags); if (!m->current_pgpath) __choose_pgpath(m, 0); - pgpath = m->current_pgpath; - - if (pgpath) { - *bdev = pgpath->path.dev->bdev; - *mode = pgpath->path.dev->mode; + if (m->current_pgpath) { + if (!m->queue_io) { + *bdev = m->current_pgpath->path.dev->bdev; + *mode = m->current_pgpath->path.dev->mode; + r = 0; + } else { + /* pg_init has not started or completed */ + r = -ENOTCONN; + } + } else { + /* No path is available */ + if (m->queue_if_no_path) + r = -ENOTCONN; + else + r = -EIO; } - if ((pgpath && m->queue_io) || (!pgpath && m->queue_if_no_path)) - r = -ENOTCONN; - else if (!*bdev) - r = -EIO; - spin_unlock_irqrestore(&m->lock, flags); if (r == -ENOTCONN && !fatal_signal_pending(current)) {
In multipath_prepare_ioctl(), - pgpath is a path selected from available paths - m->queue_io is true if we cannot send a request immediately to paths, either because: * there is no available path * the path group needs activation (pg_init) - pg_init is not started - pg_init is still running - m->queue_if_no_path is true if the device is configured to queue I/O if there is no available path If !pgpath && !m->queue_if_no_path, the handler should return -EIO. However in the course of refactoring the condition check has broken and returns success in that case. Since bdev points to the dm device itself, dm_blk_ioctl() calls __blk_dev_driver_ioctl() for itself and recurses until crash. You could reproduce the problem like this: # dmsetup create mp --table '0 1024 multipath 0 0 0 0' # sg_inq /dev/mapper/mp <crash> [ 172.648615] BUG: unable to handle kernel paging request at fffffffc81b10268 [ 172.662843] PGD 19dd067 PUD 0 [ 172.666269] Thread overran stack, or stack corrupted [ 172.671808] Oops: 0000 [#1] SMP ... This patch fixes the condition check with some clarifications. Fixes: e56f81e0b01e ("dm: refactor ioctl handling") Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Mike Snitzer <snitzer@redhat.com> --- drivers/md/dm-mpath.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)