FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 03-29-2011, 05:19 PM
Milan Broz
 
Default dm table: Reject unconfigured device

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@redhat.com>
Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
---
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;

--
1.7.4.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-29-2011, 06:22 PM
Mike Snitzer
 
Default dm table: Reject unconfigured device

On Tue, Mar 29 2011 at 1:19pm -0400,
Milan Broz <mbroz@redhat.com> 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@redhat.com>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> ---
> 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.cpen_dev()). But I like the
additional info that is naturally provided in device_area_is_invalid().

Acked-by: Mike Snitzer <snitzer@redhat.com>

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 03-29-2011, 06:33 PM
Milan Broz
 
Default dm table: Reject unconfigured device

On 03/29/2011 08:22 PM, Mike Snitzer wrote:
>
> Part of me was thinking it best to check this, and error out, as early
> as possible (e.g. in drivers/md/dm-table.cpen_dev()). But I like the
> additional info that is naturally provided in device_area_is_invalid().

Another option is change loop device by assigning request
fn early (it will reject IO if unbounded, once configured it
has request fn defined forever).

But then you get lot of errors instead of crash on access, like
Buffer I/O error on device dm-0, logical block 0

So rejecting unconfigured device use this way seems to me
like better idea.
(If there is no request function, how DM can map it?)

Milan

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 07:17 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org