On Tue, Mar 29 2011 at 1:19pm -0400,
Milan Broz <mbroz@xxxxxxxxxx> wrote:
> Some block devices exist but have request function undefined
> (e.g. loop device without backing file).
>
> Because we are allowing devices with zero size
> (offline device in multipath, see commit
> 2cd54d9bedb79a97f014e86c0da393416b264eb3, there should be
> additional check if device is initialised.
>
> This patch adds check that block device has request function
> defined, otherwise it rejects mapped area as invalid.
>
> Reproducer is trivial: dm-mirror on unbound loop device
> (no backing file on loop devices)
>
> dmsetup create x --table "0 8 mirror core 2 8 sync 2 /dev/loop0 0 /dev/loop1 0"
>
> and mirror resync will immediatelly cause OOps.
>
> BUG: unable to handle kernel NULL pointer dereference at (null)
> ? generic_make_request+0x2bd/0x590
> ? kmem_cache_alloc+0xad/0x190
> submit_bio+0x53/0xe0
> ? bio_add_page+0x3b/0x50
> dispatch_io+0x1ca/0x210 [dm_mod]
> ? read_callback+0x0/0xd0 [dm_mirror]
> dm_io+0xbb/0x290 [dm_mod]
> do_mirror+0x1e0/0x748 [dm_mirror]
>
> Signed-off-by: Milan Broz <mbroz@xxxxxxxxxx>
> Reported-by: Zdenek Kabelac <zkabelac@xxxxxxxxxx>
> ---
> drivers/md/dm-table.c | 17 +++++++++++++++++
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 38e4eb1..12caa66 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -361,6 +361,7 @@ static void close_dev(struct dm_dev_internal *d, struct mapped_device *md)
> static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
> sector_t start, sector_t len, void *data)
> {
> + struct request_queue *q;
> struct queue_limits *limits = data;
> struct block_device *bdev = dev->bdev;
> sector_t dev_size =
> @@ -369,6 +370,22 @@ static int device_area_is_invalid(struct dm_target *ti, struct dm_dev *dev,
> limits->logical_block_size >> SECTOR_SHIFT;
> char b[BDEVNAME_SIZE];
>
> + /*
> + * Some devices exist but have not yet request function defined
> + * (e.g. loop with not bounded backing file)
> + * Do not allow using such devices.
> + */
> + q = bdev_get_queue(bdev);
> + if (!q || !q->make_request_fn) {
> + DMWARN("%s: %s is not yet initialised: "
> + "start=%llu, len=%llu, dev_size=%llu",
> + dm_device_name(ti->table->md), bdevname(bdev, b),
> + (unsigned long long)start,
> + (unsigned long long)len,
> + (unsigned long long)dev_size);
> + return 1;
> + }
> +
> if (!dev_size)
> return 0;
>
Checking this in device_area_is_invalid() works and has a nice
side-effect of providing useful information about the device in
question.
Part of me was thinking it best to check this, and error out, as early
as possible (e.g. in drivers/md/dm-table.c:open_dev()). But I like the
additional info that is naturally provided in device_area_is_invalid().
Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
