From 5c675f83c68fbdf9c0e103c1090b06be747fa62c Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Mon, 15 Dec 2014 12:56:56 +1100 Subject: md: make ->congested robust against personality changes. There is currently no locking around calls to the 'congested' bdi function. If called at an awkward time while an array is being converted from one level (or personality) to another, there is a tiny chance of running code in an unreferenced module etc. So add a 'congested' function to the md_personality operations structure, and call it with appropriate locking from a central 'mddev_congested'. When the array personality is changing the array will be 'suspended' so no IO is processed. If mddev_congested detects this, it simply reports that the array is congested, which is a safe guess. As mddev_suspend calls synchronize_rcu(), mddev_congested can avoid races by included the whole call inside an rcu_read_lock() region. This require that the congested functions for all subordinate devices can be run under rcu_lock. Fortunately this is the case. Signed-off-by: NeilBrown --- drivers/md/dm-raid.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/md/dm-raid.c') diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 07c0fa0fa284..777d9ba2acad 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -746,13 +746,7 @@ static int raid_is_congested(struct dm_target_callbacks *cb, int bits) { struct raid_set *rs = container_of(cb, struct raid_set, callbacks); - if (rs->raid_type->level == 1) - return md_raid1_congested(&rs->md, bits); - - if (rs->raid_type->level == 10) - return md_raid10_congested(&rs->md, bits); - - return md_raid5_congested(&rs->md, bits); + return mddev_congested(&rs->md, bits); } /* -- cgit v1.2.3 From 3ca5a21a9c02bdebe2d95268482031f002efcf23 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 29 May 2014 11:23:23 +0300 Subject: dm raid: fix a couple integer overflows My static checker complains that if "num_raid_params" is UINT_MAX then the "if (num_raid_params + 1 > argc) {" check doesn't work as intended. The other change is that I moved the "if (argc != (num_raid_devs * 2))" condition forward a few lines so it was before the call to context_alloc(). If we had an integer overflow inside that function then it would lead to an immediate crash. Signed-off-by: Dan Carpenter Signed-off-by: Mike Snitzer --- drivers/md/dm-raid.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'drivers/md/dm-raid.c') diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c index 07c0fa0fa284..41acc9dd7342 100644 --- a/drivers/md/dm-raid.c +++ b/drivers/md/dm-raid.c @@ -1243,7 +1243,7 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) argv++; /* Skip over RAID params for now and find out # of devices */ - if (num_raid_params + 1 > argc) { + if (num_raid_params >= argc) { ti->error = "Arguments do not agree with counts given"; return -EINVAL; } @@ -1254,6 +1254,12 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) return -EINVAL; } + argc -= num_raid_params + 1; /* +1: we already have num_raid_devs */ + if (argc != (num_raid_devs * 2)) { + ti->error = "Supplied RAID devices does not match the count given"; + return -EINVAL; + } + rs = context_alloc(ti, rt, (unsigned)num_raid_devs); if (IS_ERR(rs)) return PTR_ERR(rs); @@ -1262,16 +1268,8 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) if (ret) goto bad; - ret = -EINVAL; - - argc -= num_raid_params + 1; /* +1: we already have num_raid_devs */ argv += num_raid_params + 1; - if (argc != (num_raid_devs * 2)) { - ti->error = "Supplied RAID devices does not match the count given"; - goto bad; - } - ret = dev_parms(rs, argv); if (ret) goto bad; -- cgit v1.2.3